Skip to content

fix: report installed_version and available_version states in OTA page instead of build id and date #2470

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

Merged
merged 3 commits into from
May 3, 2025

Conversation

Ricc68
Copy link
Contributor

@Ricc68 Ricc68 commented Apr 26, 2025

This PR fixes the confusion that is caused to the users by displaying manufacturer-specific build-id and build-date in OTA page, which are optional variables and not all manufacturers implement them. This causes those fields in the page to be blank.
I have replaced them with installed_version, as defined by the ZigBee protocol and available_version detected by firmware repository, which are directly related and actually used during an OTA check/update.

See also my conversation with Nerivec in Koenkk/zigbee2mqtt#24051

This PR fixes Koenkk/zigbee2mqtt#24051, Koenkk/zigbee2mqtt#24579, Koenkk/zigbee2mqtt#15839, Koenkk/zigbee2mqtt#25403, Koenkk/zigbee2mqtt#16421, #1860 and possibly others that I couldn't find in the Issues list.

Please, @nurikk, @Nerivec and @Koenkk review and let me know if it's ok or there is something that requires more work.

@Nerivec
Copy link
Contributor

Nerivec commented Apr 26, 2025

I don't think that's the expected formatting.

File version A: 0x10053519 represents application release 1.0 build 05 with stack release 3.5 b19.
File version B: 0x10103519 represents application release 1.0 build 10 with stack release 3.5 b19.
File version C: 0x10103701 represents application release 1.0 build 10 with stack release 3.7 b01.

They are just taking the plain hex and splitting it to make a version out of it.

        const versionString = version.toString(16).padStart(8, "0");
        const appRelease = `${versionString[0]}.${versionString[1]}`;
        const appBuild = versionString.slice(2, 4);
        const stackRelease = `${versionString[4]}.${versionString[5]}`;
        const stackBuild = versionString.slice(6);

@Ricc68
Copy link
Contributor Author

Ricc68 commented Apr 26, 2025

I see, yes I started with plain hex formatting as defined by the specs but then I have switched to dec for my aesthetic sense, my bad, I should have stick with the specs.
Your code is much more elegant: my code shows how my main background is coming from C 😂 and taking shortcut using a, b, c, d was also not good coding practice 🤦‍♂️

Will fix it.

@Ricc68
Copy link
Contributor Author

Ricc68 commented Apr 26, 2025

Ok I think I've got it right this time.
I have tried to keep the version string compact, but at the same time keep app and stack versions separate for better readability.
I've tried many formats and I've ended with the following:

immagine

If you like it I will commit the change, otherwise I'm open to any suggestion 👍

Or maybe another option is to pad each hex to 2 digits so with many devices all the versions will be nicely formatted in a fixed size column:

immagine

I like more this formatting, to me it is aesthetically more pleasant so if you agree I would commit this one.

@Nerivec
Copy link
Contributor

Nerivec commented Apr 26, 2025

I would say just:
0.0.00-0.0.00
No padding needed, since it would never be 2-digit (max f.f.ff-f.f.ff).
I would not display "no firmware available" if it's because the device hasn't checked yet, just "N/A".

@Ricc68
Copy link
Contributor Author

Ricc68 commented Apr 26, 2025

Now it should be ok:

immagine

@@ -2662,7 +2662,7 @@
"firmware_installed_version": "Firmware installed version",
"firmware_available_version": "Firmware available version",
"firmware_installed_version_na": "Not yet reported by device",
"firmware_available_version_na": "No firmware available",
"firmware_available_version_na": "N/A",
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't need to use a translation for this one, the rest of frontend doesn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, fix done.

@Koenkk Koenkk merged commit d76f3cc into nurikk:dev May 3, 2025
2 checks passed
@Koenkk
Copy link
Collaborator

Koenkk commented May 3, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nous A1Z smart plug: OTA page not reporting firmware version and build date
3 participants