feat(app): handle non-deterministic protocols #12173
Conversation
5693664
to
5cbcde1
Compare
Codecov Report
@@ Coverage Diff @@
## edge #12173 +/- ##
==========================================
+ Coverage 74.03% 74.05% +0.01%
==========================================
Files 2172 2172
Lines 59831 59842 +11
Branches 6145 6151 +6
==========================================
+ Hits 44297 44313 +16
+ Misses 14151 14129 -22
- Partials 1383 1400 +17
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| @@ -54,12 +60,20 @@ export function RunProgressMeter(props: RunProgressMeterProps): JSX.Element { | |||
| const lastRunCommandKey = useLastRunCommandKey(runId) | |||
| const lastRunCommandIndex = | |||
| analysisCommands.findIndex(c => c.key === lastRunCommandKey) ?? 0 | |||
| const lastRunCommandIndexFromAllCommands = | |||
There was a problem hiding this comment.
It looks like the actual value that we want is from the meta of allCommandsQuery response. It includes a totalLength key whose value we should be able to sub in for the numerator in the case that the run is non-deterministic
There was a problem hiding this comment.
just to make sure I understand then, the denominator should be a "?" and the numerator should be the totalLength of the commands: totalLength / ?
| const lastRunCommandIndexFromAllCommands = | ||
| runCommands.findIndex(c => c.key === lastRunCommandKey) ?? 0 | ||
| const { data: commandDetails } = useCommandQuery( | ||
| runId, | ||
| runCommands[lastRunCommandIndexFromAllCommands]?.id | ||
| ) |
There was a problem hiding this comment.
@b-cooper I'm now grabbing the totalLength from meta and displaying that, but I believe I still need these lines to be able to access the command text from the run rather than the analysis as I'm currently doing in L 89-96, correct?
ed80b78
to
2d8319c
Compare
No problem, am I good to merge this then, or is there still some dev testing required? |
* edge: refactor(protocol-engine): Shorten error docstrings (#12230) chore(app): update electron-builder version from v24 alpha to v24 (#12233) chore(app): add new icons for ODD hi-fi (#12231) docs: more release docs (#12226) fix(app, odd): check isOnDevice true instead of if it is null (#12228) docs: remove references to make bump in releasing (#12222) refactor(app): scroll to current split from jump to tick, qol tweaks to run preview (#12218) fix(app): rewire cli and jupyter snippets to offsets modals (#12180) feat(app): handle non-deterministic protocols (#12173)
Overview
When a protocol run is non-deterministic we now hide the jump to the current step button, hide the progress meter, and only display the total count of commands in the run to avoid any confusion or misdirection.
closes RLAB-299
Test Plan
Tested locally during development with automated tests and by short-circuiting some logic to force a "non-deterministic state"
Changelog
RunPreviewto only show when a run is deterministicRunProgressMeterwhen a protocol is non-deterministictotal-run-commands/?when a protocol is non-deterministicRunProgressMetercomponentReview requests
Please give this a run on a robot, I am not able to test this in action using only the Sim.
Risk assessment
fairly low, just UI changes for edge cases