Skip to content
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

feat(api): show air gap volume in simulate log #17569

Merged
merged 1 commit into from
Feb 21, 2025
Merged

feat(api): show air gap volume in simulate log #17569

merged 1 commit into from
Feb 21, 2025

Conversation

ddcc4
Copy link
Contributor

@ddcc4 ddcc4 commented Feb 21, 2025

Overview

When simulate-ing a protocol, the logs just show this for an AIR_GAP command:

Air gap
Logs from this command:
INFO (publisher): command.AIR_GAP: 

whereas the other commands have useful info. I want to see how much air we're aspirating for the air gap. After this change, the logs will look like this:

Air gap of 11.5 uL
Logs from this command:
INFO (publisher): command.AIR_GAP: instrument: Flex 1-Channel 50 µL on right mount, volume: 11.5, height: None

Test Plan and Hands on Testing

I'm mainly relying on the CI tests. I did run simulate with this change and checked the output manually.

Review requests

I am not too familiar with this part of the code.

  • Why is the implementation of the simulate logger in legacy_commands? What does "legacy" refer to?
  • I don't quite understand how arguments are passed to the functions in legacy_commands/commands.py. How was the code able to work when the function was declared as def air_gap() with no arguments before this change?

Risk assessment

Medium? I don't know if there are consumers of the simulate log who expect the payload never to change.

@ddcc4 ddcc4 requested review from sfoster1 and sanni-t February 21, 2025 17:33
@ddcc4 ddcc4 requested a review from a team as a code owner February 21, 2025 17:33
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

Thank you!

@ddcc4 ddcc4 merged commit 5f5e3bc into edge Feb 21, 2025
30 checks passed
@ddcc4 ddcc4 deleted the dc-sim-airgap branch February 21, 2025 18:31
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