Tecan spark by xbtu2 · Pull Request #798 · PyLabRobot/pylabrobot

xbtu2 · 2025-12-21T01:33:06Z

tecan spark WIP. luminescence coming up soon. i had a hard time to get io.usb to work with this, so i'm directly using libusb for now

hazlamshamin · 2025-12-21T05:56:39Z

CRAZY COOOOL! this is VERY complete

Copilot

Pull request overview

This PR adds support for the Tecan Spark plate reader by implementing a comprehensive async backend with USB communication, packet parsing, and data processing capabilities. The implementation includes controls for various instrument subsystems and processors for absorbance and fluorescence measurements.

Key Changes

New async USB reader with packet parsing for Tecan Spark devices
Absorbance and fluorescence data processors with real-world test coverage
Comprehensive control modules for plate transport, optics, sensors, measurements, and system configuration
Utility function for calculating non-overlapping well rectangles

Reviewed changes

Copilot reviewed 23 out of 24 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pylabrobot/plate_reading/utils.py	Adds helper to find non-overlapping rectangles covering plate wells
pylabrobot/plate_reading/tecan/spark20m/spark_reader_async.py	Implements async USB communication layer for Spark devices
pylabrobot/plate_reading/tecan/spark20m/spark_packet_parser.py	Parses binary measurement data packets from the device
pylabrobot/plate_reading/tecan/spark20m/spark_processor.py	Processes raw measurement data into absorbance/fluorescence values
pylabrobot/plate_reading/tecan/spark20m/spark_backend.py	Main backend integrating all components for plate reading operations
pylabrobot/plate_reading/tecan/spark20m/controls/*.py	Control modules for device subsystems (optics, sensors, movement, etc.)
pylabrobot/plate_reading/tecan/spark20m/*_tests.py	Unit tests for reader, processor, and backend components

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

pylabrobot/plate_reading/tecan/spark20m/controls/system_control.py

pylabrobot/plate_reading/tecan/spark20m/spark_reader_async.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

rickwierenga · 2026-01-21T00:30:42Z

is this ready for review?

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

- Remove AutoReaderProxy, integrate reading into send_command - Remove reading context manager (no longer needed) - Rename control classes to follow PEP8 (BaseControl, PlateControl, etc.) - BaseControl now takes send_command directly instead of reader - Make init_read and get_response private methods - Add io.binary.Reader utility class Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

rickwierenga · 2026-01-31T18:49:05Z

Code review changes

Simplified the Spark control architecture:

Removed AutoReaderProxy - This was a magic proxy that wrapped every async method call with a reading context. Instead, the reading logic is now integrated directly into send_command(), making the flow more explicit and easier to follow.
Removed reading() context manager - No longer needed since send_command() handles read setup internally.
Renamed control classes to follow PEP8 - baseControl → BaseControl, plateControl → PlateControl, measurement_control → MeasurementControl, etc.
Simplified BaseControl - Now takes send_command function directly instead of the entire reader object, since that's all it uses.
Made internal methods private - init_read() → _init_read(), get_response() → _get_response() since they're only used internally by send_command().
Added io.binary.Reader - Utility class for binary parsing, used to replace the custom _read_u8, _read_u16, etc. helper functions in the packet parser.

# Conflicts: # pylabrobot/io/binary.py # pylabrobot/io/usb.py

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Replace custom byte parsing helpers (_read_bytes, _read_u8, _read_u16, _read_u32, _read_string) with the Reader class from pylabrobot.io.binary. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Use pytest.approx() instead of exact equality for float comparisons in test_process_real_data tests. Compare row by row since pytest.approx does not support nested data structures. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Add max_workers parameter to USB class constructor with default of 1 to maintain backward compatibility. SparkReaderAsync passes max_workers=16 for its specific needs. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

The PR fixed the timeout conversion for the main write but missed the zero-length packet write. PyUSB expects timeout in milliseconds, so timeout must be multiplied by 1000. Also add max_workers parameter to USBValidator for signature compatibility. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Replace manual calculation of well spacing with the new item_dx and item_dy properties from ItemizedResource. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

pylabrobot/plate_reading/tecan/spark20m/spark_reader_async.py

rickwierenga · 2026-02-01T04:09:26Z

instead of returning False on error, can we raise the error in send command? this is a more pythonic way of dealing with errors

The previous implementation used an attempt counter (default 10000) which could spin for 100+ seconds with 10ms sleeps. Now uses time.monotonic() to enforce a wall-clock deadline (default 60 seconds), making the timeout behavior predictable and bounded. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Use _safe_div() for the (raw_ref_signal - ref_dark) denominator to return NaN instead of raising ZeroDivisionError when reference signal equals reference dark value. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

MirrorCarrier, LaserPowerState, and LightingState were already defined in spark_enums.py. Import them instead of redefining. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

…trol These enums are already defined in spark_enums.py. Import them instead. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

- Add explicit None check for inner_mult_index in _parse_nested_mult call - Import ScanDirection from spark_enums instead of re-exporting through measurement_control Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

rickwierenga · 2026-02-05T20:00:56Z

is it a problem that DataControl is not complete yet? I'm fine either way

xbtu2 · 2026-02-06T23:20:05Z

how does it look now?

rickwierenga · 2026-02-07T05:04:39Z

this uses the io dev directly in the backend. we would rather route that stuff through IO

is it impossible to use add an endpoint: Optional = None to _read_packet and _write that would override self.read_endpoint and write_endpoint if specified

…ark_reader_async.py

rickwierenga · 2026-03-09T18:08:39Z

in SparkReaderAsync._get_response, elif parsed.get("type") == "RespError": raises an error but this is caught by the following except statement and just logged. what should happen there? I think either return the error or raise it?

rickwierenga · 2026-03-09T18:15:37Z

SparkDevice.LUMINESCENCE and SparkDevice.PLATE_TRANSPORT are missing the read_data key

…read_data endpoints - Introduce SparkError exception so RespError packets propagate to callers instead of being silently swallowed by broad except handlers - Add read_data endpoint keys to PLATE_TRANSPORT and LUMINESCENCE device entries in DEVICE_ENDPOINTS - Update test to assert SparkError is raised on RespError

…ssing device - Add dynamic USB endpoint discovery via _discover_endpoints() to support different Spark hardware variants (e.g. 0x81 vs 0x82 bulk-in endpoints) - Add test_spark_hardware.py with connection, measurement, and transport tests - Add SPARK_SPECTROMETER to SparkDevice enum - Add require_device decorator for early failure when hardware is not connected - Fix unused variable lint warning in send_command

- Add missing human_readable_device_name arg to USB() constructor - Set up mock devices dict in test so device-connected checks pass Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Fix ruff formatting in usb.py (lambda line wrapping) - Guard usb.core/usb.util imports in spark_reader_async.py with try/except - Guard SparkBackend import in plate_reading/__init__.py for envs without USB Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Match the established pattern from PR PyLabRobot#941: optional dependency errors should be deferred to instantiation time, not silently swallowed at import time. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Mark the Tecan Spark backend as experimental and replace the interactive hardware test script with a proper docs notebook. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

rickwierenga · 2026-03-22T22:58:46Z

thanks!

xbtu2 added 5 commits December 20, 2025 12:23

tecan spark backend

b132be7

use enum for endpoints

fbec36f

add time

16edc58

format

ad9843d

fix typo

a799855

BioCam requested a review from Copilot December 21, 2025 07:33

Copilot AI reviewed Dec 21, 2025

View reviewed changes

pylabrobot/plate_reading/tecan/spark20m/controls/system_control.py Outdated Show resolved Hide resolved

pylabrobot/plate_reading/tecan/spark20m/spark_reader_async.py Outdated Show resolved Hide resolved

xbtu2 and others added 6 commits December 21, 2025 11:24

Update pylabrobot/plate_reading/tecan/spark20m/spark_reader_async.py

4190426

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

remove unused method

c937c24

use plr usb io

7560ed3

Merge branch 'main' into tecan_spark

1e2c30a

fix test

0c99649

Merge branch 'main' into tecan_spark

eaf2f3b

rickwierenga and others added 2 commits January 31, 2026 10:18

Use io.binary.Reader in spark_packet_parser

24ec263

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Merge remote-tracking branch 'origin/main' into tecan_spark

b56ffdd

# Conflicts: # pylabrobot/io/binary.py # pylabrobot/io/usb.py

rickwierenga force-pushed the tecan_spark branch from fc9cf36 to b56ffdd Compare January 31, 2026 19:20

rickwierenga and others added 7 commits January 31, 2026 11:27

Rename control attributes to include _control suffix

aaa6fae

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Refactor spark_packet_parser.py to use binary.py Reader

9747549

Replace custom byte parsing helpers (_read_bytes, _read_u8, _read_u16, _read_u32, _read_string) with the Reader class from pylabrobot.io.binary. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Make USB max_workers configurable per-instance

2e1db3c

Add max_workers parameter to USB class constructor with default of 1 to maintain backward compatibility. SparkReaderAsync passes max_workers=16 for its specific needs. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Merge remote-tracking branch 'origin/main' into tecan_spark

31949fa

Use item_dx and item_dy properties in SparkBackend

847d895

Replace manual calculation of well spacing with the new item_dx and item_dy properties from ItemizedResource. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

rickwierenga reviewed Jan 31, 2026

View reviewed changes

pylabrobot/plate_reading/tecan/spark20m/spark_reader_async.py Outdated Show resolved Hide resolved

fix send_commend and tests

e95d88d

rickwierenga and others added 5 commits February 5, 2026 11:52

Protect fluorescence RFU calculation from division by zero

ab79ec1

Use _safe_div() for the (raw_ref_signal - ref_dark) denominator to return NaN instead of raising ZeroDivisionError when reference signal equals reference dark value. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Remove duplicate enum definitions from optics_control.py

707f5b1

MirrorCarrier, LaserPowerState, and LightingState were already defined in spark_enums.py. Import them instead of redefining. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Remove duplicate ScanDirection and ScanDarkState from measurement_con…

f78e07d

…trol These enums are already defined in spark_enums.py. Import them instead. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Fix type annotation issues for mypy --strict

6d74c4d

- Add explicit None check for inner_mult_index in _parse_nested_mult call - Import ScanDirection from spark_enums instead of re-exporting through measurement_control Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

xbtu2 added 2 commits February 6, 2026 14:44

refactor io/usb.py

6179ef9

fix duplicate commands

8c147f4

fix import order

77927c9

xbtu2 added 2 commits February 7, 2026 12:26

add endpoint override in io/usb.py; refactor read/write methods in sp…

45f3158

…ark_reader_async.py

fix import

3a3cc1c

xbtu2 and others added 6 commits March 10, 2026 07:50

Merge branch 'main' into tecan_spark

43ee628

Fix test failure and mypy error in Spark backend

38938bc

- Add missing human_readable_device_name arg to USB() constructor - Set up mock devices dict in test so device-connected checks pass Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Merge branch 'main' into tecan_spark

e5bc326

rickwierenga force-pushed the tecan_spark branch from 57e2c0a to e5bc326 Compare March 22, 2026 06:41

rickwierenga and others added 6 commits March 22, 2026 00:00

Fix CI: only serialize USB endpoint addresses when set

3a727cd

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Fix CI: sort imports and skip USB tests when pyusb not installed

a2a5282

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Rename SparkBackend to ExperimentalSparkBackend, add docs notebook

f42c44a

Mark the Tecan Spark backend as experimental and replace the interactive hardware test script with a proper docs notebook. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add tecan-spark to plate-reading toctree

ce93eeb

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Fix import sorting in plate_reading __init__

e20f152

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

rickwierenga merged commit 23cb24f into PyLabRobot:main Mar 22, 2026
21 checks passed

Conversation

xbtu2 commented Dec 21, 2025

Uh oh!

hazlamshamin commented Dec 21, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

rickwierenga commented Jan 21, 2026

Uh oh!

rickwierenga commented Jan 31, 2026

Code review changes

Uh oh!

Uh oh!

rickwierenga commented Feb 1, 2026

Uh oh!

rickwierenga commented Feb 5, 2026

Uh oh!

xbtu2 commented Feb 6, 2026

Uh oh!

rickwierenga commented Feb 7, 2026

Uh oh!

rickwierenga commented Mar 9, 2026

Uh oh!

rickwierenga commented Mar 9, 2026

Uh oh!

Uh oh!

rickwierenga commented Mar 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants