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(pipettes): add acceleration to TipActionRequest message #653

Merged
merged 21 commits into from
Jul 27, 2023

Conversation

caila-marashaj
Copy link
Contributor

@caila-marashaj caila-marashaj commented May 3, 2023

This code accompanies the monorepo changes in this pr- it adds acceleration to TipActionRequest messages, and uses the position_flags parameter in GearMotorAcks after a move is complete

@caila-marashaj caila-marashaj changed the title add acceleration to TipActionRequest message feat(pipettes): add acceleration to TipActionRequest message May 3, 2023
@caila-marashaj caila-marashaj marked this pull request as ready for review May 3, 2023 15:15
@caila-marashaj caila-marashaj requested a review from a team May 3, 2023 15:15
@codecov-commenter
Copy link

codecov-commenter commented May 3, 2023

Codecov Report

Merging #653 (323147b) into main (7d7c1ae) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #653   +/-   ##
=======================================
  Coverage   84.30%   84.30%           
=======================================
  Files          87       87           
  Lines        3836     3836           
=======================================
  Hits         3234     3234           
  Misses        602      602           
Impacted Files Coverage Δ
include/can/core/messages.hpp 99.22% <ø> (ø)
include/motor-control/core/motor_messages.hpp 100.00% <ø> (ø)
...ttes/core/tasks/gear_move_status_reporter_task.hpp 100.00% <ø> (ø)
...rol/core/stepper_motor/motor_interrupt_handler.hpp 99.58% <100.00%> (ø)

Copy link
Contributor

@Laura-Danielle Laura-Danielle left a comment

Choose a reason for hiding this comment

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

You need to edit:

include/motor-control/core/stepper-motor/motion_controller.hpp::PipetteMotionController

in order for the acceleration to actually get used (and I believe the pip motion controller task too).

Let me know if you can't find where these are.

Copy link
Contributor

@Laura-Danielle Laura-Danielle left a comment

Choose a reason for hiding this comment

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

yep looks good, but please don't merge in till the python pr is ready to be merged in.

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.

just in case so we don't drive ourselves crazy if we flash the wrong thing, can we put the acceleration part at the end of the message and set it to 0 if we don't have a long enough message to include it

@sfoster1
Copy link
Member

sfoster1 commented May 5, 2023

Looks good otherwise though!

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! Really great work.

@caila-marashaj caila-marashaj merged commit 4693209 into main Jul 27, 2023
38 of 40 checks passed
@caila-marashaj caila-marashaj deleted the add-acc-to-tip-action-msg branch July 27, 2023 16:34
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.

None yet

5 participants