Improving hardwareId behavior #2335
Conversation
src/js/protocols/stm32usbdfu.js
Outdated
| @@ -775,6 +775,7 @@ STM32DFU_protocol.prototype.upload_procedure = function (step) { | |||
| for (var j = 0; j < self.flash_layout.sectors[i].num_pages; j++) { | |||
| if (self.options.erase_chip) { | |||
| // full chip erase | |||
| $('span.hardwareId').hide(); | |||
There was a problem hiding this comment.
I think the data driven approach is better - we want to reset the data (with FC.resetState()) when it becomes invalid (disconnect of the device), and not 'combat the symptoms' by hiding the element that displays cached invalid data.
There was a problem hiding this comment.
@mikeller Yes, it makes sense, I wasn't really sure which was the best option, done!
Fix indendation Add css changes Change hiden to reset data aproach Add semicolons
asizon
force-pushed
the
improve_hardwareId_check
branch
from
7353b40 to
1647ca6
Compare
| @@ -90,9 +90,6 @@ PortHandler.check_usb_devices = function (callback) { | |||
| callback(self.dfu_available); | |||
| } | |||
| if (!$('option:selected', self.portPickerElement).data().isDFU) { | |||
| if (!GUI.connected_to) { | |||
There was a problem hiding this comment.
Not sure why you want to remove this - to me it seems to be the right approach to reset the FC state whenever a disconnect / change of the selected port is detected.
There was a problem hiding this comment.
Because when configurator reads hardwareId its not in Dfu mode, then it reset data and hardwareId never shown when is flashing. Im not sure how to fix this behavior.
There was a problem hiding this comment.
Maybe I can add a condition to aply only if not in flash progress
There was a problem hiding this comment.
Ok, I can see what you are trying to do - the current implementation is correct (because there is a disconnect when DFU mode is entered), but it is not super helpful, as the hardware id does not show during flashing.
Maybe I can add a condition to aply only if not in flash progress
Yes, I think this makes sense - if flashing is in progress then we know that a 'disconnect' is not a physical disconnect.
There was a problem hiding this comment.
I got it!Definitely I have removed the reset when chip erase is checked, as you said in the past it can be usefull if the user selects wrong target.
src/js/protocols/stm32usbdfu.js
Outdated
| @@ -775,6 +775,7 @@ STM32DFU_protocol.prototype.upload_procedure = function (step) { | |||
| for (var j = 0; j < self.flash_layout.sectors[i].num_pages; j++) { | |||
| if (self.options.erase_chip) { | |||
| // full chip erase | |||
| FC.resetState(); | |||
There was a problem hiding this comment.
Calling a high level function from inside a protocol implementation is problematic - especially since this is happening only in a very specific code path. If this is really the correct approach, it will have to be done with a callback.
There was a problem hiding this comment.
Okey im going to look at this because I think this is the correct approach to can show hide hardwareId when is really needed.
|
Kudos, SonarCloud Quality Gate passed!
|
So now, hardwareId is disabled when flash finished or if Full erase chip is enabled. Also added flex functionality to fix hardwareId position.