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

Update UI when reconnected to Launcher and a port is selected in the port list #572

Open
PropGit opened this issue Apr 16, 2021 · 8 comments
Assignees
Labels
Beta Issue The issue is reported in a beta release bug Something isn't working

Comments

@PropGit
Copy link
Collaborator

PropGit commented Apr 16, 2021

A recent enhancement, to improve the user experience despite websocket disconnects, causes the UI's download, terminal, and graph buttons to remain ghosted when they can actually be active.

Symptom:

  • Solo and Launcher are running and connected
  • A port was selected and downloads occurred (all is well)
  • Launcher was closed (by accident or otherwise)
  • Launcher was reopened
  • Solo continues to show the selected port, and maintains the port list (this is the intentional enhancements mentioned above)
  • The pink warning message disappears and goes back to the normal connected state
  • The download to RAM, download to EEPROM, Terminal, and Graph buttons are disabled.
    All connections are good and the user's preferred port is indeed selected, but they can not download.

The user can indeed get beyond this by selecting a different port (even a blank one) from the port list and then switch back to their desired port, causing the UI to update properly.

Of course, it'd be best if the UI updater routine recognized that Launcher has just re-connected and a port is indeed already selected in the list, so it can enable the ghosted buttons.

@PropGit PropGit added bug Something isn't working Beta Issue The issue is reported in a beta release labels Apr 16, 2021
@PropGit
Copy link
Collaborator Author

PropGit commented Apr 16, 2021

After writing this I realized that there must be something else happening that I'm not aware of, because the mentioned enhancements was specifically to allow seamless UI usage despite a momentary disconnect/reconnect, and that has definitely been tested and proven to work.

The ghosting issue was only noticed when Launcher was closed and sometime later reopened.

@zfi zfi self-assigned this Apr 16, 2021
@zfi zfi added this to the Release v1.5.8 milestone Apr 16, 2021
@zfi
Copy link
Contributor

zfi commented Apr 16, 2021

The Load to Ram/EEprom buttons should be visible and active if a) the web socket is connected (active), there is at least one serial port in the port list and the first element of the port list is non-blank.

The initial description indicates that the user's selected port survives the Launcher restart. When the Launcher is restarted, the toolbar controller is called as soon as a connection is established. It is also called when

  • The socket connection is closed or lost
  • Solo receives a message from the socket
  • When a port list is received from the socket
  • When the user selects a com port

I think the most likely explanation is that the toolbar controller is not getting called at one of these points and for an unknown reason. I am unable to replicate the error with a single device connected to a Mac. Perhaps this only happens when there are multiple devices connected to the computer?

@PropGit
Copy link
Collaborator Author

PropGit commented Apr 16, 2021

I will try to replicate here as I have multiple ports.

@PropGit
Copy link
Collaborator Author

PropGit commented Apr 16, 2021

After more testing, I see two possible solutions, though the second is preferred:

The Load to Ram/EEprom buttons should be visible and active if a) the web socket is connected (active), there is at least one serial port in the port list and the first element of the port list is non-blank.

It appears to me that the "non-blank" item condition is the cause of the continued-ghosted button issue.

Tests:
Initial condition (BPL connected and a port has been selected by user):
image

Terminate BPL, then start BPL (BPL connected and the users's previously selected port is still "selected," but buttons are ghosted)
image

... this is because the new instance of the launcher doesn't know a preferred port was previously (and still is) selected, so it issues the port list with a blank initial item followed by current ports on the system.
image

If the user selects the blank port item (just to select a different one) and then re-selects their desired port, the buttons activate again (ie: a preferred port was selected and Solo sent a preferred port message to the Launcher, so Launcher issues further lists with the preferred port first (instead of blank).

Solution 1 (not preferred)
Change Solo's logic from
"...and the first element of the port list is non-blank"
to
"...and the selected element of the port list is non-blank."

Solution 2 (preferred)
Change Solo's logic so that upon a Launcher connection...

  • Immediately check the UI's port list (or the internal portlist object?) and, if the selected item is non-blank then send a preferred-port-message to Launcher with the text of the selected item
  • Send a port-list-message to initiate reception of port list updates (which will contain the preferred port first).

@zfi
Copy link
Contributor

zfi commented Apr 16, 2021

The second set of images is concerning. We changed the port refresh behavior recently such that Solo will retain the last know port list and selected port between web socket disconnects.

In the second set of images, the toolbar code is looking at the internal port list, not the UI element. This internal list does not change until Solo receives a new port list from Launcher. At that moment, Solo clears the internal port list and then builds a new one from the data received from Launcher. This process also preserves the selected port in Solo's world view. I believe that is why we see COM1 in the port drop down instead of the blank that Launcher is sending in slot 0.

The next step after reconnect is a toolbar update. The current code is looking at the internal port list to see if it has any entries and, if it does, is the first entry non-blank. The logic here is that we cannot load code to the device without a valid port. This is why the LoadTo buttons are grayed out. I agree that we should actually be looking at the selected port to determine if there is one and it's not an empty entity. In other words, do we have a device to talk to.

This does raise one more issue. Solo is assuming that the selected port that it retained between web socket resets is still a valid port. A new port has not actually been selected and Launcher is unaware of a selected port, so there is a possibility that Solo could try to use a port that may not exist. I need to figure out how Launcher learns what the selected port is - possibly by observing the port when a program load is received from Solo.

@zfi
Copy link
Contributor

zfi commented Apr 16, 2021

I inserted a cluster of console logs and discovered that things are not what they seem to be. Specifically, the code that is setting the preferred com port was not notifying the clientService object. This is why the internal preferred port was never set. The second issue is that the clientService setPreferred port would always send a notification to the Launcher, even if the preferred com post had not changed.

I will issue a new PR shortly that addresses these concerns.

@PropGit
Copy link
Collaborator Author

PropGit commented Apr 16, 2021

I see. Thank you.

Launcher will take any port Solo tells it to connect to and will first look through it's list of known ports for a match. If it finds a match, it moves on with the requested process. If it doesn't find a match, it will reply back with an error indicating it can't find the requested port. I'm concerned that there may be one place where that logic isn't followed, so I'll verify now.

@zfi zfi modified the milestones: Release v1.5.8, Release v1.5.9 Apr 21, 2021
@zfi
Copy link
Contributor

zfi commented Apr 29, 2021

@PropGit I have been testing this with the latest build to 1.5.9-RC1 and the buttons appear to be very responsive after the launcher is reloaded. Solo now persists the port list internally and no longer clears it when the connection to the Launcher is lost. On reconnect, Solo now only needs to see an active web socket connection to activate the buttons - because of the retained port list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Beta Issue The issue is reported in a beta release bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants