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(hardware): add csv file logging capability to capacitive_probe method #14785

Merged
merged 23 commits into from
Jun 5, 2024

Conversation

pmoegenburg
Copy link
Contributor

@pmoegenburg pmoegenburg commented Apr 3, 2024

Overview

This PR, along with Opentrons/ot3-firmware#768, adds csv file logging capability to the capacitive_probe ot3api method. This work utilizes the work that adds the same capability to the liquid_probe process. The MoveGroupSingleAxisStep, AddSensorLinearMoveRequest, and SendAccumulatedSensorDataRequest classes were modified to include a sensor_type data field. The capacitive_probe_ot3_tunable script was added for testing.

Test Plan

  • Ran script without csv logging and detected saltwater liquid height
  • Ran script with csv loffing and detected saltwater liquid height

Changelog

Review requests

Risk assessment

@pmoegenburg pmoegenburg added feature Ticket is a feature request / PR introduces a feature firmware hardware embedded labels Apr 3, 2024
@pmoegenburg pmoegenburg self-assigned this Apr 3, 2024
@pmoegenburg pmoegenburg requested a review from a team as a code owner April 3, 2024 03:20
Copy link

codecov bot commented Apr 3, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 10 lines in your changes missing coverage. Please review.

Project coverage is 60.15%. Comparing base (ccc944e) to head (22e65ec).
Report is 122 commits behind head on edge.

Current head 22e65ec differs from pull request most recent head ca7ef59

Please upload reports for the commit ca7ef59 to get more accurate results.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             edge   #14785       +/-   ##
===========================================
- Coverage   92.43%   60.15%   -32.29%     
===========================================
  Files          77      189      +112     
  Lines        1283    10587     +9304     
===========================================
+ Hits         1186     6369     +5183     
- Misses         97     4218     +4121     
Flag Coverage Δ
g-code-testing 92.43% <ø> (ø)
hardware 55.70% <83.33%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
.../opentrons_hardware/firmware_bindings/constants.py 99.29% <100.00%> (ø)
.../firmware_bindings/messages/message_definitions.py 97.62% <100.00%> (ø)
...ns_hardware/firmware_bindings/messages/messages.py 91.66% <ø> (ø)
...ns_hardware/firmware_bindings/messages/payloads.py 96.04% <100.00%> (ø)
...ware/opentrons_hardware/hardware_control/motion.py 83.14% <100.00%> (ø)
...ons_hardware/hardware_control/move_group_runner.py 92.12% <100.00%> (ø)
...rdware/opentrons_hardware/sensors/sensor_driver.py 82.69% <ø> (ø)
...pentrons_hardware/hardware_control/tool_sensors.py 85.71% <80.76%> (ø)

... and 104 files with indirect coverage changes

@pmoegenburg pmoegenburg requested a review from a team as a code owner April 9, 2024 19:31
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.

Looking good but definitely some stuff to change.

.gitignore Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
api/src/opentrons/config/defaults_ot3.py Outdated Show resolved Hide resolved
api/src/opentrons/config/defaults_ot3.py Outdated Show resolved Hide resolved
api/src/opentrons/config/types.py Outdated Show resolved Hide resolved
@pmoegenburg pmoegenburg requested a review from sfoster1 May 7, 2024 17:44
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.

Looks good to me, nice!

Copy link
Contributor

@ryanthecoder ryanthecoder left a comment

Choose a reason for hiding this comment

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

LGTM

@pmoegenburg pmoegenburg merged commit b2c6390 into edge Jun 5, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
embedded feature Ticket is a feature request / PR introduces a feature firmware hardware
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants