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 upload and serial #661

Merged
merged 5 commits into from
Dec 7, 2021
Merged

Fix upload and serial #661

merged 5 commits into from
Dec 7, 2021

Conversation

fstasi
Copy link
Contributor

@fstasi fstasi commented Dec 3, 2021

Why

This PR fixes #615 by refactoring part of the logic involved when creating and managing the serial connection.

How

How it worked before

  • the IDE browser (Frontend) was managing part of the state of the serial connections. It was using and array of open widgets (namely Serial Monitor & Serial Plotter) to track the need for a connection to the Serial Port. It was requesting the BE to create/destroy the actual serial connection based on the number of widgets in the array.
  • it also used an internal variable to keep track of the state of the serial connection
  • when the "upload sketch" button was pressed, the FE was requesting the BE to close the serial connection, toggle its internal connection status, do the upload, and, once finished, resume the serial connection and it's toggle it's internal connection status again.
  • Since the actual state of the serial connection was not known this caused a number of issues including, but not limited to, the fact that it was not able to restore the flag correctly. The latter caused the infamous bug Subsequent uploads fail after uploading to Leonardo w/ Serial Monitor/Plotter open #615.

How it works now

We completely re-designed how the connection mechanism works, moving the vast majority of the business logic from the FE to the BE

  • websocket connections are tracked in the BE only. When the number of connections is > 0 (aka: a widget is requesting data) a serial connection is opened and data is forwarded to the widgets). When there are no websocket connections the BE safely closes the serial connection as it's not needed anymore.
  • the frontend does not store the state of the serial connections anymore.
  • when the upload button is pressed, the backend closes the serial connection (but the websocket connections are still open). When the upload finishes the BE communicates the FE it's done. The FE restores the upload button and request the BE to re-open the serial connection
  • the BE re-open the serial connection if the number of open websockets is still > 0

Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

I tested with the 7d10e89 build and it fixes #615 for me.

I am still able to reproduce #615 with the nightly build.

👍

@per1234
Copy link
Contributor

per1234 commented Dec 6, 2021

I notice that this change makes #613 persist even after one of the windows was closed:

  1. Upload a sketch that prints unique data periodically to two different boards. For example:
    int x;
    void setup() {
      Serial.begin(9600);
    }
    void loop() {
      Serial.print("Board1:");
      Serial.println(x++);
      delay(500);
    }
    and:
    int x;
    void setup() {
      Serial.begin(9600);
    }
    void loop() {
      Serial.print("Board2:");
      Serial.println(x++);
      delay(500);
    }
  2. Select the port of one board (I'll refer to it as "board 1") in one IDE window (I'll refer to it as "window 1").
  3. Open Serial Monitor in window 1.
  4. Open another IDE window (I'll refer to it as "window 2").
  5. Select the port of the other board (I'll refer to it as "board 2") in window 2.
  6. Open Serial Monitor in window 2.
    Note that the Serial Monitor in each window is displaying a mixture of the output from both boards:
    Board1:42
    Board2:1
    Board1:43
    Board2:2
    Board1:44
    Board2:3
    Board1:45
    Board2:4
    [...]
    
  7. Close one of the windows (I'll use window 2 for the sake of this demo).

Note that the other window is still displaying the mixture of the output from both boards. The same applies to Serial Plotter.

Prior to the changes made in this PR, the mixed serial output behavior introduced by #597 ceased once one port was closed, but after this change the mixed output state persists and attempts to upload to board 2 fail with an "Access is denied" error.

@fstasi fstasi force-pushed the fix-upload-and-serial branch 3 times, most recently from 2bca1e1 to 9d2d8ee Compare December 7, 2021 13:11
Copy link
Contributor

@AlbyIanna AlbyIanna left a comment

Choose a reason for hiding this comment

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

LGTM 🐕

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.

Subsequent uploads fail after uploading to Leonardo w/ Serial Monitor/Plotter open
3 participants