-
Notifications
You must be signed in to change notification settings - Fork 122
io.Socket refactor
#753
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
io.Socket refactor
#753
Conversation
… the Backend abstractmethods
…unndant method, chenge default ip address, change test_get_base assertion to tuple, add sleep to test_set_power
…lidation - Updated get_signal to correctly parse response and return signal value. - Modified get_location_z_clearance to return z_world as a boolean. - Enhanced set_location_z_clearance to convert z_world to an integer for command. - Improved get_location_config and set_location_config to handle bit mask configurations with validation checks.
…refactor speed handling and location management
…onfiguration tests
…inates; update method signatures and add new backend classes for PreciseFlex 400 and PreciseFlex 3400
…file. Correct profile get error for 'straight' property.
… Arm class to be almost a copy of the ArmBackend
…e operations - Introduced VerticalAccess and HorizontalAccess data classes to define access patterns. - Updated approach, pick_plate, and place_plate methods to accept access patterns. - Improved documentation for methods to clarify usage and default behaviors.
There was a problem hiding this 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 refactors the io.Socket class to improve the API design by distinguishing between different read patterns and switching from string-based to bytes-based I/O operations.
Key Changes:
- Split read operations into specialized methods:
read(),readline(),readuntil(), andread_until_eof()instead of a singleread()method with flags - Changed write and read methods to work with
bytesinstead of strings, requiring explicit encoding/decoding at call sites - Updated validation data storage to use hex format for binary safety
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| pylabrobot/io/io.py | Updated abstract base interface to specify bytes type for write data parameter and read return type |
| pylabrobot/io/socket.py | Major refactor: removed read_once parameter, added new read methods (readline, readuntil, read_until_eof), changed I/O to use bytes, updated logging/capture to use hex encoding, added read/write locks |
| pylabrobot/thermocycling/thermo_fisher/thermo_fisher_thermocycler.py | Adapted to new Socket API by encoding writes to bytes, decoding reads from bytes, and using read_until_eof() instead of read(read_once=False) |
| pylabrobot/thermocycling/thermo_fisher/proflex_tests.py | Updated test mocks to return bytes instead of strings, added separate mock for read_until_eof(), updated expected write calls to use bytes |
Comments suppressed due to low confidence (1)
pylabrobot/thermocycling/thermo_fisher/thermo_fisher_thermocycler.py:370
- The exception handling on line 369 catches
TimeoutError, but with the new Socket implementation, read operations raiseasyncio.TimeoutErrorinstead ofTimeoutError. This means timeouts will not be caught by this except block and will propagate as unhandled exceptions. Either update the Socket methods to raiseTimeoutError, or update this handler to catchasyncio.TimeoutError.
except TimeoutError:
return ""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
xbtu2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
Thanks Bo |
read_oncein read, instead split intoreadandread_until_eof(not all higher level protocols require/support this)read_untilandreadline