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): Add G Code for pipette config in driver #3388

Merged
merged 21 commits into from
May 2, 2019

Conversation

Laura-Danielle
Copy link
Contributor

@Laura-Danielle Laura-Danielle commented Apr 26, 2019

overview

Closes #3324 and closes #3323. This will allow for configurations to be modified at run time/upon detection of a different pipette type.

changelog

  • Detects when a new model is loaded by checking the model name for v2
  • Adds driver commands to update both the steps per mm as well as the pipette configs listed in update_pipette_config

review requests

  • Requires this firmware change, pls review that before reviewing this PR.
  • Couldn't come up with a good way to test M365.* gcodes without physically testing on the robot, any ideas?

@Laura-Danielle Laura-Danielle requested a review from a team April 26, 2019 15:23
@codecov
Copy link

codecov bot commented Apr 26, 2019

Codecov Report

Merging #3388 into edge will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge    #3388      +/-   ##
==========================================
+ Coverage   52.57%   52.59%   +0.01%     
==========================================
  Files         775      775              
  Lines       22905    22980      +75     
==========================================
+ Hits        12043    12086      +43     
- Misses      10862    10894      +32
Impacted Files Coverage Δ
app/src/discovery/selectors.js 100% <100%> (ø) ⬆️
opentrons/drivers/smoothie_drivers/driver_3_0.py 77.47% <0%> (-2.23%) ⬇️
opentrons/legacy_api/robot/robot.py 87.08% <0%> (-0.6%) ⬇️
opentrons/config/robot_configs.py 98.34% <0%> (ø) ⬆️
opentrons/legacy_api/instruments/pipette.py 95.03% <0%> (+0.01%) ⬆️
opentrons/hardware_control/simulator.py 89.18% <0%> (+0.19%) ⬆️
opentrons/legacy_api/robot/mover.py 89.01% <0%> (+0.63%) ⬆️
opentrons/drivers/smoothie_drivers/__init__.py 69.56% <0%> (+2.89%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4feee9...d984125. Read the comment docs.

@@ -267,6 +267,16 @@ def get_lights(self) -> Dict[str, bool]:
self._config.instrument_offset[mount.name.lower()],
instrument_data['id'])
self._attached_instruments[mount] = p
if 'v2' in model:
Copy link
Member

Choose a reason for hiding this comment

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

What happens if you replace a v2 with a v1.x? Don't we need to reset the steps per mm?

api/src/opentrons/hardware_control/__init__.py Outdated Show resolved Hide resolved
self._driver.update_steps_per_mm({axis: 2133.33})
# TODO(LC25-4-2019): Modify configs to update to as
# testing informs better values
self._driver.update_pipette_config(axis, {'home': 172.15})
Copy link
Member

Choose a reason for hiding this comment

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

here we need to update the pipette config back to defaults for non-v2 models (or take the time now to add this data to the model configs)

expected['Z'] = 450
assert smoothie.steps_per_mm == expected


def test_set_acceleration(smoothie, monkeypatch):
from opentrons.drivers import serial_communication
from opentrons.drivers.smoothie_drivers import driver_3_0
Copy link
Member

Choose a reason for hiding this comment

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

We should add tests to make sure that the proper gcodes are sent for the 365s, and also that the proper driver methods are called upon pipette detection

Laura-Danielle and others added 12 commits April 30, 2019 19:03
- We need to set the max travel for pipette plunger axes appropriately for the
  pipette type so the smoothie knows the correct distance to home (fixes homing
  error)
- we have to recalculate the axis maximum every time in mover since it might
  change now based on what kind of pipette is loaded (this takes 1ms or so)
- the smoothie firmware file had one more character in its name than stored
  internally so it would always think it had to update
@Laura-Danielle Laura-Danielle merged commit 77fffa6 into edge May 2, 2019
@Laura-Danielle Laura-Danielle deleted the api-driver-add-g-code-config branch May 2, 2019 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Affects the `api` project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pipette+: Making homing details modifiable at run-time Pipette+: Load motion configs based on pipette model
2 participants