Skip to content

Conversation

@hazlamshamin
Copy link
Contributor

@hazlamshamin hazlamshamin commented Dec 20, 2025

  • Add Tecan Infinite 200 PRO backend + tests.
  • Supports absorbance and fluorescence, including masking to read specified list of wells, up to 384-well plate.
  • Luminescence is experimental due to lack of glowing sample for testing.
  • Make USB capture decoding resilient to malformed escape sequences.

EDIT: all three modes now reflect what OEM does with 100% accuracy, and luminescence is no longer experimental ([e92c6db])

Tests: make lint, make format-check, make typecheck, make test.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a backend for the Tecan Infinite 200 PRO plate reader (M Plex series), adding support for absorbance and fluorescence measurements with experimental luminescence support. The implementation includes configurable well masking for reading specific wells on plates up to 384-well format.

Key changes:

  • New TecanInfinite200ProBackend class with USB communication and measurement decoding
  • Specialized decoders for absorbance, fluorescence, and luminescence measurement streams
  • Enhanced USB capture error handling to gracefully manage malformed escape sequences

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
pylabrobot/plate_reading/tecan_infinite_backend.py Implements the complete backend with transport layer, measurement decoders, and plate reading methods for all three modes
pylabrobot/plate_reading/tecan_infinite_backend_tests.py Comprehensive unit tests for decoders, scan geometry, and ASCII frame handling
pylabrobot/plate_reading/init.py Exports the new backend and configuration class for public API access
pylabrobot/io/usb.py Adds error handling to decode operations to prevent crashes from malformed USB data

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

if end == -1:
return False, None
text = buffer[1:end].decode("ascii", "ignore")
del buffer[: end + 2]
Copy link

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This deletion assumes the buffer has at least end + 2 bytes, but only checks for bytes up to end. If the buffer ends exactly at ETX (index end), accessing end + 1 would be valid, but end + 2 could exceed buffer length. Add a length check before deletion.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have pushed a new commit for extra guard for this after checking the hexdumps. additionally, there has already been guard for absorbance and fluorescence which checked that the consumed

i pushed a new commit (ca73508) for extra guard for this after checking the hexdumps. found cases where ETX arrives at end‑of‑packet (though this does not appear to change the measurement data). nonetheless the extra guard avoids consuming partial frames. also note in current code, we already validate measurement frames: ABS checks wavelength (words[1]), FI checks excitation/emission (words[1]/[2]). only LUM has no wavelength field to match.

Copy link
Member

@rickwierenga rickwierenga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apologies for the late response, I was working on slas and then the old nimbus PR

I have two small initial questions / comments for things I think we can parameterize easily

"TIME 0,LAG=0",
"TIME 0,READDELAY=0",
"GAIN 0,VALUE=100",
"POSITION 0,Z=20000",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  "POSITION 0,Z=20000",

do you know what these units are? would be nice to parameterize them as focal_height

rickwierenga and others added 3 commits January 20, 2026 16:10
- USB._read_packet() now accepts optional size to read exact bytes from wire
- USB.read() passes remaining byte count to _read_packet() when size specified
- PyUSBInfiniteTransport.read() uses new size parameter instead of slicing
- USBValidator.read() updated for interface consistency

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove InfiniteTransport protocol and PyUSBInfiniteTransport class
- TecanInfinite200ProBackend creates USB instance internally as self.io
- Move reset logic into backend's _recover_transport method
- Simplifies the transport layer by removing unnecessary abstraction

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@rickwierenga
Copy link
Member

could you please explain what _MODE_CAPABILITY_COMMANDS are?

are they just more parameters like this?

      f"BEAM DIAMETER={self._capability_numeric('ABS', '#BEAM DIAMETER', 700)}",

that we can add as commands? why not add them?

@rickwierenga rickwierenga mentioned this pull request Jan 21, 2026
rickwierenga and others added 5 commits January 20, 2026 19:30
Replace custom _u16be, _u32be, _i32be helper functions with the Reader
class from io/binary using little_endian=False for big-endian parsing.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
rickwierenga and others added 6 commits January 20, 2026 20:03
Remove redundant step_loss local variable and parameter since _end_run
can access self._active_step_loss_commands directly.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Inline command lists in _configure_absorbance, _configure_fluorescence,
  and _configure_luminescence with direct _send_ascii calls
- Refactor _cleanup_protocol to use a local helper function instead of
  building a commands list

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Rename:
- _send_ascii -> _send_command
- _frame_ascii_command -> _frame_command
- _drain_ascii -> _drain
- _read_ascii_response -> _read_command_response
- _ascii_parser -> _parser

Also update related docstrings.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Tests verify that open, close, read_absorbance, read_fluorescence, and
read_luminescence send the correct USB commands in the correct order.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
rickwierenga and others added 7 commits January 24, 2026 11:13
Reduces duplication across read_absorbance, read_fluorescence, and
read_luminescence methods. Also removes unused _active_mode field.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Callers now compute ordered_wells and scan_wells once and pass
ordered_wells to _run_scan directly.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The "Prepare" naming came from the device command "PREPARE REF" but
didn't convey what the data actually represents. These structures
contain calibration data (dark/bright references, gain values) used
to convert raw measurements into calibrated values.

Renamed:
- _AbsorbancePrepare -> _AbsorbanceCalibration
- _FluorescencePrepare -> _FluorescenceCalibration
- _LuminescencePrepare -> _LuminescenceCalibration
- Related functions, properties, and variables

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use distinct variable names (cal/data) instead of reusing 'decoded'
for both calibration and measurement data decoding to avoid mypy
type conflicts.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The "marker" name was misleading - it's actually the binary payload
length extracted from protocol frames like "18,BIN:". The length
happens to identify packet type (different packet types have
characteristic sizes), but calling it "marker" obscured its true
meaning.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Comment on lines +975 to +977
async def _begin_run(self) -> None:
await self._initialize_device()
self._reset_stream_state()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason the device would un-initialize? every command calls _begin_run, which calls _initialize_device. but _initialize_device is already called on setup.

Comment on lines +38 to +41

def _integration_value_to_seconds(value: int) -> float:
return value / 1_000_000.0 if value >= 1000 else value / 1000.0

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain this?

rickwierenga and others added 3 commits January 24, 2026 14:40
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Lazily initialize the asyncio.Lock to avoid "no current event loop"
error on Python 3.9, which requires an event loop when creating locks.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants