- 
                Notifications
    You must be signed in to change notification settings 
- Fork 186
feat(shared-data, compoents): add tracking commands to runlog #19935
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
base: edge
Are you sure you want to change the base?
Conversation
| Codecov Report❌ Patch coverage is  
 Additional details and impacted files@@            Coverage Diff             @@
##             edge   #19935      +/-   ##
==========================================
+ Coverage   24.23%   24.65%   +0.41%     
==========================================
  Files        3540     3539       -1     
  Lines      298345   298225     -120     
  Branches    39863    39861       -2     
==========================================
+ Hits        72298    73517    +1219     
+ Misses     226029   224690    -1339     
  Partials       18       18              
 Flags with carried forward coverage won't be shown. Click here to find out more. 
 🚀 New features to boost your workflow:
 | 
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.
Yeah this looks basically right to me, thanks! Just small adjustments.
        
          
                components/src/assets/localization/en/protocol_command_text.json
              
                Outdated
          
            Show resolved
            Hide resolved
        
      | @@ -72,6 +80,14 @@ export const getPipettingCommandText = ({ | |||
| flow_rate: flowRate, | |||
| }) | |||
| } | |||
| case 'aspirate_while_tracking': { | |||
| return t('aspirate_while_tracking', { | |||
| track_from_location: trackFromLocation, | |||
| track_to_location: trackToLocation, | |||
| volume, | |||
| flow_rate: flowRate, | |||
| }) | |||
| } | |||
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.
A few things:
- I think this casevalue needs to beaspirateWhileTrackinginstead ofaspirate_while_tracking, to match what Protocol Engine puts in thecommandType.
- I would move the trackFromLocationandtrackToLocationvariable declarations inside thecaseblock. That lets TypeScript narrow the type ofcommandbased on the fact thatcommandType === 'aspirateWhileTracking'. It will be able to see thatcommand.params.trackFromLocationandcommand.params.trackToLocationwill always exist, which lets you delete thetrackFromLocation in command.paramscheck.
- Reminder to add a similar block for dispenseWhileTracking.
27153fe    to
    896de1a      
    Compare
  
    
Overview
add the typescript hooks for the aspirate/dispense while tracking commands