Skip to content

Fix Hardware Id hiding when no version detected #2266

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Dec 8, 2020
Merged

Fix Hardware Id hiding when no version detected #2266

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b518509
Hide hardwareId when no version detected
asizon Nov 4, 2020
5b836e0
Force FC status reset when flashing
asizon Dec 7, 2020
95665de
Move resetState to stm32.js
asizon Dec 8, 2020
67be783
Added condition to avoid duplication when is connected
asizon Dec 8, 2020
883bb9c
Remove hardwareId when checks for usb devices
asizon Dec 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/betaflight-logo/BetaflightLogo.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
{{ firmwareId }}
</span>
<br />
<span v-if="hardwareId">
<span v-if="firmwareVersion && hardwareId">
Copy link
Member

@mikeller mikeller Nov 4, 2020

Choose a reason for hiding this comment

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

This is a bit of a workaround that relies on certain conditions to be true on something unrelated firmwareVersion. As such it is likely to be broken without being detected in the future. It is probably a more maintainable approach to reset hardwareId to be empty at the same place that firmwareVersion is reset on disconnect.

Copy link
Member

@McGiverGim McGiverGim Nov 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think this will be as easy...
I've looked at the code, and it seems this two FC/MSP variables are being filled by two different MSP commands. The problem is that we have a listener that each time we connect, including when we connect to go to DFU to flash a firmware, we read the hardwareId. The other one is only read when we really "connect" in the Configurator and then we read all the FC info.
With all of this:

  • Do we want to add this info to the logo when flashing or only when connected? If we want when flashing too we need to add a listener to onDisconnect or something similar if exists to erase the information.
  • If we want only when connected, another option, more descriptive, is to pass a variable to the Vue object indicating if we want to add FC info to the logo, that can be the associated to the variable that contains the info about if we are connected or not.

EDIT: this variable can be GUI.connected_to that contains false if not connected or the port in text form if connected. I don't know if there is other more suitable.

Copy link
Member Author

@asizon asizon Nov 7, 2020

Choose a reason for hiding this comment

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

Yes @mikeller I'm not satisfied enough with this fix that is based on firmwareVersion, I'll give it one more review to find a better solution

Copy link
Member

@mikeller mikeller Nov 7, 2020

Choose a reason for hiding this comment

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

Sorry, I found the time to test this with an actual board only now - since the hardwareId is based on actual information that is returned from the board, I am not sure that it is wrong to show it while flashing the firmware. After that it becomes stale, as there is no way for us to ensure that the same board is connected, especially with the flashing process involving changes between the VCP in serial mode and DFU.
So maybe, instead of hiding this when flashing, we should just make sure that hardwareId is reset at the end of flashing?
Maybe this could be improved optically by moving the Target information above the firmware version information in the top bar.

Copy link
Member Author

@asizon asizon Nov 8, 2020

Choose a reason for hiding this comment

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

@mikeller definitelly it happens when dfu autoenable is used in firmware flasher tab, fc reboots in dfu mode and the hardwareId is recognized. If we enable dfu in setup tab al works as expected.

Copy link
Member

@mikeller mikeller Nov 8, 2020

Choose a reason for hiding this comment

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

@asizon: Yes, this is to be expected. If the flight controller is not in DFU mode already when the flashing is started, we try to 'half-connect' in order to issue the MSP command to reboot the flight controller into DFU mode. This is when the hardware id is read. So keeping this hardware id while the subsequent flashing process is running seems to me to make sense.

Copy link
Member

@mikeller mikeller Nov 15, 2020

Choose a reason for hiding this comment

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

@asizon: Do you want to change this so that the hardware id is shown during flashing if it is known?

Copy link
Member Author

@asizon asizon Nov 15, 2020

Choose a reason for hiding this comment

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

Yes @mikeller im going to look at this, sorry

Copy link
Member

@mikeller mikeller Nov 15, 2020

Choose a reason for hiding this comment

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

No reason to be sorry - this is a hobby, and everything is voluntary.

{{ $t("versionLabelTarget") }}: {{ hardwareId }}
</span>
</span>
Expand Down