NabuCasa / universal-silabs-flasher

Flashes Silicon Labs radios running EmberZNet or CPC multi-pan firmware
GNU General Public License v3.0
83 stars 16 forks source link

fix: Remove hardcoded `FirmwareImageType` in favor of a plain string #80

Open Nerivec opened 2 days ago

Nerivec commented 2 days ago

https://github.com/NabuCasa/universal-silabs-flasher/blob/2e302a2a63c15e91437663e132240730399d90a2/universal_silabs_flasher/firmware.py#L111 This line hard-fails if the in-file firmware type is anything other than the values in FirmwareImageType, preventing flashing of otherwise perfectly valid, and supported firmware. Since that metadata is added by firmware builder manifest, it should not prevent the use of custom values, or even more "official" values that are not yet entered in the enum (forcing update of flasher).

puddly commented 2 days ago

I'd like to keep the firmware types restricted to a set of well-known values (since there will be at most only a few). If you want to add internal information to describe a firmware variant, I think adding a new free-form field to the JSON like variant would be better.

https://github.com/NabuCasa/silabs-firmware-builder/pull/82 will rename all of the firmware types to be more generic (e.g. zigbee_ncp) as opposed to just the SDK project name, which should simplify naming.

Nerivec commented 2 days ago

It prevents the use of other firmware types like the RCP blehci, even though the flasher should support it just fine out-of-the-box.

Also the error is very generic (ValueError: 'ncp-uart-sw' is not a valid FirmwareImageType) since it's derived from the above line, and let users wonder what exactly is behind it.

puddly commented 2 days ago

Yeah, I'll definitely change it to a warning instead of an error. Just going forward, I think we should stick to a standard firmware type instead of tweaking it per firmware variant.