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

fix(app-shell,app): Send labware files and runtime parameters over USB #14994

Merged
merged 9 commits into from
Apr 25, 2024

Conversation

SyntaxColoring
Copy link
Contributor

@SyntaxColoring SyntaxColoring commented Apr 24, 2024

Overview

Fixes RESC-247 and RQA-2627.

Test Plan

  • Following the steps in RESC-247, make sure protocols with custom labware work over USB now.
  • Following the steps in RQA-2627, make sure runtime parameters work over USB now.
  • Make sure there's no discernable app freeze or hiccup that wasn't there before when you click the "Send" button, especially with a big protocol.
  • Make sure other requests (like toggling the lights) are unaffected.
  • Make sure requests over Wi-Fi are unaffected.

Changelog

When the Electron rendering process wants to send a USB message, it needs to do so through the Electron main process. This is a problem because some JavaScript types for those messages—Files and Blobs and FormDatas—can't be sent over IPC. We've worked around this for POST /protocols requests by substituting a protocolKey: string in place of these problematic types, in the IPC message. The main process then finds that and reconstructs the Files and FormDatas from it.

That implementation didn't account for labware files external to the protocol (RESC-247), nor did it account for new multipart fields for things like run-time parameters. (RQA-2627).

With this PR, the rendering process slurps all the file contents—protocol and labware—into memory, and then sends them over IPC. We still need to use a substitute structure for the FormData, and the main process still needs to reconstruct all of this back into FormDatas and Files. But the reconstruction code no longer needs to be concerned with application-level ideas like "labware" and "protocols" and where those things live on the filesystem, which is intended to make all of this a bit more robust.

Review requests

See my review comments.

Risk assessment

Medium.

app-shell/src/usb.ts Outdated Show resolved Hide resolved
@SyntaxColoring SyntaxColoring marked this pull request as ready for review April 24, 2024 21:51
@SyntaxColoring SyntaxColoring requested a review from a team as a code owner April 24, 2024 21:51
@SyntaxColoring SyntaxColoring changed the title fix(app-shell,app): Send labware files over USB fix(app-shell,app): Send labware files and runtime parameters over USB Apr 24, 2024
The actual requirements turn out to be stricter than supporting the structured clone algorithm. For example, `Blob` is structured-cloneable, but doesn't work with Electron's IPC.
Copy link
Member

@shlokamin shlokamin left a comment

Choose a reason for hiding this comment

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

nice work, the value.arrayBuffer() conversion was very clever thinking

@SyntaxColoring SyntaxColoring merged commit 7bb202e into edge Apr 25, 2024
21 checks passed
@SyntaxColoring SyntaxColoring deleted the resc_247 branch April 25, 2024 14:55
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

2 participants