pybricks-code icon indicating copy to clipboard operation
pybricks-code copied to clipboard

Keep the hub name as a default when installing/re-installing PyBricks Firmware

Open afarago opened this issue 1 year ago • 3 comments

Initial implementation for keeping the last connected hub id in memory. pybricks/support/#1415 Show it as the suggested name in the firmware dialog.

Next steps:

  • keep the hub name and id (or even collect all connected hubs and names) in browser storage
  • keep the hub name in hub's flash

afarago avatar Jul 17 '24 18:07 afarago

  • keep the hub name and id (or even collect all connected hubs and names) in browser storage

See other uses of useLocalStorage() for how to do this.

There currently isn't a way to get a unique identifier for each hub, so the best we can do is just store the last name entered and/or the name of the last hub connected. (Maybe there is a way to do this for USB hubs with the serial number, but I think it is more important that the UX is the same for all hubs)

  • keep the hub name in hub's flash

Not sure what you mean by this. It is already written to flash when when firmware is flashed.

Next steps:

We'll also want coverage tests for this.

dlech avatar Jul 17 '24 19:07 dlech

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 72.49%. Comparing base (36ae3e0) to head (150b096). Report is 8 commits behind head on master.

Files Patch % Lines
...re/installPybricksDialog/InstallPybricksDialog.tsx 85.71% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2297      +/-   ##
==========================================
+ Coverage   72.47%   72.49%   +0.01%     
==========================================
  Files         201      201              
  Lines        5126     5133       +7     
  Branches     1086     1089       +3     
==========================================
+ Hits         3715     3721       +6     
- Misses       1374     1375       +1     
  Partials       37       37              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jul 18 '24 16:07 codecov[bot]

  • keep the hub name and id (or even collect all connected hubs and names) in browser storage

See other uses of useLocalStorage() for how to do this.

There currently isn't a way to get a unique identifier for each hub, so the best we can do is just store the last name entered and/or the name of the last hub connected. (Maybe there is a way to do this for USB hubs with the serial number, but I think it is more important that the UX is the same for all hubs)

  • keep the hub name in hub's flash

Not sure what you mean by this. It is already written to flash when when firmware is flashed.

Next steps:

We'll also want coverage tests for this.

Thank you David for the review! I have moved the logic to local storage, moving the code to InstallPybricksDialog.tsx.

The current precedence is as

  1. user input in the textbox
  2. last connected device name
  3. empty

The upside of this approach is that if user does not enter explicit value, connecting to a different device changes the target name. The downside of this is that if you ever connect to a hub, you will not be able to get back to the empty name. LMK if that is acceptable.

Coverage tests I am owing, trying to craft the tests running altogether. Yet my WSL it is still giving errors. WIP.


keep the hub name in hub's flash

Not sure what you mean by this. It is already written to flash when when firmware is flashed.

The hub does have a current value, yet as it enters DFU mode we are not aware of it anymore. The best alternative in the future imo would be that if user do not enter any name we would read and reuse this before the flashing. @dlech LMK if you plan to add anything on that.

afarago avatar Jul 18 '24 16:07 afarago