Skip to content

Inheco ODTC#894

Open
cmoscy wants to merge 63 commits intoPyLabRobot:mainfrom
cmoscy:odtc
Open

Inheco ODTC#894
cmoscy wants to merge 63 commits intoPyLabRobot:mainfrom
cmoscy:odtc

Conversation

@cmoscy
Copy link
Contributor

@cmoscy cmoscy commented Feb 15, 2026

ODTC backend. Runs on-device protocols, and uses a scratch protocol name for uploading and executing individual commands like set mount temperature.

…er init

Extract parsing utilities (_extract_dict_path, _extract_xml_parameter) and
refactor 5 methods to use them. Add state verification after Initialize.
Remove redundant logs and simplify error handling.
…sistency

Split interface into send_command (result) and start_command (handle); rename backend get_method_by_name/list_method_names to get_method/list_methods; standardize docstrings and update README/notebook.
point, backend implements ThermocyclerBackend) instead of the original custom
layout; improves consistency with PyLabRobot and gives a clear recommended API.

- Rename odtc_xml → odtc_model; remove odtc.py; add ODTCThermocycler as preferred
  resource. Use computed estimated duration (PreMethod 10 min, Method from steps);
  stop parsing duration from device.
- Backend/thermocycler interface updates; pass estimated duration through all
  execution paths.
- README: Recommended Workflows (run by name, round-trip for thermal performance,
  set_block_temperature); clarify preserve temps when reusing ODTC config (only
  durations/cycle counts safe to change); trim redundancy.
…ess. Implement Get_Step,Cycle,Remaining methods. Cleanup.
@Stonelinks
Copy link

Honestly, we are using it in a pretty simplistic fashion right now (single thread wait for a protocol to finish executing), so no haven't noticed any issues with client parallelism. Also very new to pylabrobot in general.

@cmoscy
Copy link
Contributor Author

cmoscy commented Mar 16, 2026

have you noticed the client side parallelism check being necessary? It adds some complexity to code. I am not opposed to including more client side checks when this is necessary for safe device operation, but this can also be a risk if we are more stringent than what the machines requires.

  1. Don't think not including the client side check would break anything, totally fine if we want ODTC to just let the ODTC handle it by returning the error code 4 instead.

More as a source of documentation since the spec is disclosed here. But doesn't mean it has to be wired directly into execute_command

TD_SILA_FWCommandSet (25210).pdf

Move shared SiLA concepts (SiLAState, _current_state, _update_state_from_status,
_complete_pending, _post_command, start_command, _on_http event dispatch) to
InhecoSiLAInterface. ODTC overrides only what differs: _on_data_event,
_complete_pending (lock/parallelism cleanup), send_command/start_command
(state allowability, parallelism checks, lockId auto-injection).

Delete _execute_command, PendingCommand, polling fallback, FirstEventType enum,
COMMAND_FIRST_EVENT_TYPE dict, _validate_lock_id, _get_terminal_state. Net -550 lines.

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

prefer to have them handle it if it works, less for us to maintain :) error messages should be equally descriptive

@cmoscy
Copy link
Contributor Author

cmoscy commented Mar 16, 2026

prefer to have them handle it if it works, less for us to maintain :) error messages should be equally descriptive

Sure! Feel free to pull it out, or would you like me to make the corresponding changes/test?

@rickwierenga
Copy link
Member

@rickwierenga
Copy link
Member

Sure! Feel free to pull it out, or would you like me to make the corresponding changes/test?

no worries I can take care of it :) we have both the 96 and 384 well odtc. thanks for offering :)

rickwierenga and others added 3 commits March 15, 2026 17:45
…roperty

Each backend method (reset, open_door, lock_device, etc.) now calls
_run_async_command directly with ODTCCommand enum instead of routing
through execute(). Remove ODTCCommand.EXECUTE_METHOD dispatcher; extract
_execute_method_impl. Add event_receiver_uri property to base interface
so reset() no longer needs the URI as a parameter.

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

Remove variant field from ODTCHardwareConstraints (caller already knows it).
Inline get_constraints. Default ODTCProtocol.name to SCRATCH_PROTOCOL_NAME
so callers don't need fallback logic. Delete resolve_protocol_name function.
Rename local `odtc` variables to `protocol` or `odtc_protocol` for clarity.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Delete odtc_method_to_protocol (ODTCConfig return unused), merge into
odtc_protocol_to_protocol which now returns Protocol directly.
Merge estimate_odtc_protocol_duration_seconds into estimate_method_duration_seconds.
Inline _get_method_only_by_name, _stored_to_odtc_protocol, _read_opt_attr,
list_method_names, list_premethod_names. Delete dead get_premethod_by_name.
Rename odtc params to odtc_protocol in model.py.

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

what is the difference between ODTCMethodSet vs ProtocolList?

- _get_local_ip: reuse _get_link_local_interfaces from discovery for
  169.254.x.x devices (UDP routing trick picks wrong interface on
  multi-homed hosts)
- send_command: wrap await fut with asyncio.wait_for to prevent
  permanent hang when ResponseEvent never arrives
- _setup_full_path: poll GetStatus until device leaves startup before
  sending Reset (device rejects Reset during boot)
- Remove STARTUP from Reset allowed states (device rejects it)
- _on_http: skip non-dict event payloads from soap_decode
- Move SiLAError, SiLATimeoutError to base; ODTC imports from base
- Remove duplicate SOAP_RESPONSE_ErrorEventResponse
- _check_state_allowability raises directly with power-cycle hint for inError
- Allow GetStatus/GetDeviceIdentification/Reset in error states

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

cmoscy commented Mar 16, 2026

ProtocolList

Level of effort. ProtocolList is just going to pull stored method names for reference (or running an existing protocol by name). Where ODTCMethodSet will parse them into their respective ODTCProtocol objects (for detailed information, using as a starting point to edit and send as a new protocol, etc).

rickwierenga and others added 12 commits March 16, 2026 12:50
- _get_local_ip: reuse _get_link_local_interfaces for 169.254.x.x
- HTTP server: always return 200 with SOAP response, never 500
  (500 causes device to go inError)
- _on_http: skip non-dict event payloads from soap_decode
- send_command: 60s timeout for non-ExecuteMethod commands,
  lifetime_of_execution for ExecuteMethod only
- Remove startup polling (Reset works in startup)
- Allow Reset/GetStatus in all states
- Move SiLAError/SiLATimeoutError to base class
- _check_state_allowability raises with power-cycle hint for inError

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove _current_state, _update_state_from_status, and all local state
transitions (IDLE↔BUSY, →INERROR, →ERRORHANDLING). The device reports
its own state via StatusEvents and GetStatus — no need to mirror it.

Add request_status() to InhecoSiLAInterface as the single method for
querying device state (returns SiLAState). Move code-9 handling
(state query + power-cycle hint) into the base class and delete the
ODTC override. Update SCILABackend and ODTCBackend to delegate to it.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PLR never sends PrepareForInput/PrepareForOutput — we always use
OpenDoor/CloseDoor. Remove the normalization layer and the aliases
from STATE_ALLOWABILITY.

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

ProtocolList was always derived from ODTCMethodSet and list_protocols/list_methods
were trivial wrappers around get_method_set. Moved get_method_by_name into
ODTCMethodSet.get(), added __str__ for nice display.

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

send_command always waits now. Only execute_method uses asyncio.create_task
for non-blocking execution. Simple commands (open_door, close_door, reset,
etc.) no longer accept wait parameter. Timeout and lock tracking moved to
callers instead of being hardcoded in send_command.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
_post_command now returns raw (decoded, return_code, message) without
interpreting. send_command handles success codes (1, 2, 3) and delegates
errors to _handle_error_code (renamed from _handle_return_code).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Removed _check_parallelism, _executing_commands tracking. Kept
PARALLELISM_TABLE and STATE_ALLOWABILITY as reference documentation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
send_command_async sends command and returns (future, request_id) without
waiting. For sync commands (return_code 1), future is already resolved.
send_command just awaits the future with a timeout. Lock/unlock and lockId
injection moved to base InhecoSiLAInterface. ODTC send_command override
and _complete_pending deleted.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
run_protocol, execute_method, run_stored_protocol all return None now.
Backend tracks _current_request_id and _current_protocol directly.
Callers use is_method_running(), get_current_step_index(), stop_method(),
wait_for_method_completion() instead of a handle object.

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

- run_protocol calls set_block_temperature before executing method so
  device has matching start temperatures (required by ODTC hardware)
- wait=True awaits the SiLA future instead of polling is_method_running
  (premethods hold temperature forever so polling never returns)
- ODTCProtocol.datetime defaults to generate_odtc_timestamp(), name
  defaults to SCRATCH_PROTOCOL_NAME via protocol_to_odtc_protocol
- XML parsers use elem.attrib[] for required fields (fail loud)
- Removed **kwargs from open_lid, close_lid, set_block_temperature

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Delete open_door/close_door (inline into open_lid/close_lid)
- Delete wait_for_method_completion, wait_for_completion_by_time,
  _report_progress_once, _run_progress_loop_until (use TC.wait_for_profile_completion)
- Delete _get_effective_lifetime, replace lifetime_of_execution with simple timeout param
- Delete progress_log_interval/progress_callback from constructor
- Delete SCRATCH_PROTOCOL_NAME constant, add is_scratch bool on ODTCProtocol
- Simplify get_hold_time/get_*_index/get_*_count to return 0 when idle
- Document backend spec: return 0 when no profile is running
- Trim verbose docstrings on unsupported methods

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Delete open_door/close_door, inline into open_lid/close_lid
- Delete wait_for_method_completion and related progress machinery
- Replace lifetime_of_execution with simple timeout parameter
- Add is_scratch bool on ODTCProtocol, delete SCRATCH_PROTOCOL_NAME
- execute_method takes ODTCProtocol directly
- Delete _upload_odtc_protocol, merge into upload_protocol
- upload_protocol only accepts ODTCProtocol
- run_protocol accepts Protocol or ODTCProtocol, auto-runs premethod
- Delete get_default_config, instantiate ODTCConfig directly
- Freeze ODTCConfig, remove _validate flag and constraints property
- Delete validate_volume_fluid_quantity (was validating its own output)
- _current_protocol is always ODTCProtocol (no isinstance checks)
- get_hold_time/get_*_index/get_*_count return 0 when idle
- Backend spec: document return 0 when no profile running

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

should we trust ODTCConfig or ODTCProtocol defaults?

@rickwierenga
Copy link
Member

Level of effort. ProtocolList is just going to pull stored method names for reference (or running an existing protocol by name)

ODTCMethodSet could easily provide the list of names, so I deleted ProtocolList

@rickwierenga
Copy link
Member

does build_progress_from_data_event typically get "current_step_index" etc. in the payload? what does this depend on? the conditional parsing smells suspicious

@rickwierenga
Copy link
Member

in general do you know how rigid the data stream is? many dicts say .get rather than just direct access. I think we should be able to trust the data / if there are differences explain them and handle explicitly

…o base SiLA

File split (model -> protocol -> xml, no circular imports):
- odtc_model.py: dataclasses, constants, field metadata
- odtc_protocol.py: protocol conversion, progress, stage/step helpers
- odtc_xml.py: XML serialization and parsing

Move DataEvent storage/retrieval to base InhecoSiLAInterface:
- _data_events_by_request_id, _on_data_event, get_data_events
- ODTC override adds progress logging only

Other:
- build_progress_from_data_event replaces ODTCProgress.from_data_event
- _parse_data_event_payload raises on bad data instead of returning None
- payload parameter is now required (not Optional)

Co-Authored-By: Claude Opus 4.6 (1M context) <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.

3 participants