-
Notifications
You must be signed in to change notification settings - Fork 77
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
Refactor communication and protocol #180
Conversation
b5daff7
to
f553e13
Compare
Reviewer's Guide by SourceryThis pull request refactors the communication protocol and command structure of the PSLab firmware. It introduces a new packet handler for managing communication between the PSLab and the host, and updates all instruments and helper functions to use the new protocol. The new protocol includes a header with command code and payload size, mandatory acknowledgements, and a single entry point for communication. The changes also include input validation, status return values, and removal of debug messages. Sequence diagram for sending a command to the PSLab devicesequenceDiagram
participant Host
participant PacketHandler
participant Instrument
Host->>PacketHandler: Send command with header (command code, payload size) and payload
activate PacketHandler
PacketHandler->>PacketHandler: Validate header and payload size
alt Validation fails
PacketHandler-->>Host: Send error acknowledgement
else Validation succeeds
PacketHandler->>Instrument: Call instrument function with payload
activate Instrument
Instrument-->>PacketHandler: Return status and response data
deactivate Instrument
PacketHandler->>PacketHandler: Prepare response packet
PacketHandler-->>Host: Send acknowledgement with response data
end
deactivate PacketHandler
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @bessman - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 5 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 5 issues found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
The current debug method is too fragile and does not integrate well with the new protocol.
The previous model of atomic read/write functions does not work with the new protocol. Host must now explicitly mount drive, open file, repeatedly write blocks of data, close file unmount drive.
cc9c152
to
d77b7a8
Compare
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
This is now theoretically finished. It needs to be thoroughly tested, and even when everything works as intended the communication is of course completely incompatible with the current pslab-python & pslab-android. They will need to be updated to use the new protocol, and then new releases need to be synchronized. |
This requires further testing, but I need it for tomorrow's workshop so here goes. |
Rework communication to happen in a single place: src/transport/packet_handler.c
New protocol has two main differences from the old one:
Remaining modules to update to new protocol:
debug