Skip to content

Fix warning and offset voltage corruption after fw update or downgrade (#466 fix)#458

Merged
ligenxxxx merged 2 commits intohd-zero:mainfrom
Combinacijus:main
Nov 21, 2024
Merged

Fix warning and offset voltage corruption after fw update or downgrade (#466 fix)#458
ligenxxxx merged 2 commits intohd-zero:mainfrom
Combinacijus:main

Conversation

@Combinacijus
Copy link
Contributor

Fixes #446 settings corruption bug.
I tested upgrade and downgrade both on emulator and goggles successfully
Now after firmware upgrade Warning Cell Voltage and Voltage Calibration will be reinitialized to default values and saved as calibration_offset_mv and voltage_mv in setting.ini.

From now on setting.ini will contain both new and old settings:
image

Should be good to merge

@Master92
Copy link
Contributor

Couldn't you implement some sort of migration so that when I upgrade, I don't have to re-set my limits?

if (ini_haskey("power", "calibration_offset", SETTING_INI)) {
    const long oldSetting = ini_getl("power", "calibration_offset", g_setting_defaults.power.calibration_offset, SETTING_INI);
    ini_puts("power", "calibration_offset", NULL, SETTING_INI); // Deletes the key/value from the file as per documentation 
    ini_puts("power", "calibration_offset_mv", oldSetting * 100, SETTING_INI); // I'm unsure about the factor, adjust as needed
}

and the same for voltage.

@Combinacijus
Copy link
Contributor Author

@Master92 I could but I I'm not sure if it's ok to write migration code just for a single firmware upgrade which will be kept for all subsequent versions because you can't know from which old version user will upgrade
Also it will use default values when downgrading and same default when upgrading again if no extra code is added to check for already existing *_mv setting

But I also see that such migration code will save all the users hassle of setup

@ligenxxxx what's your thoughts on this? Is such migration code ok? If so I can make a new commit

@ligenxxxx
Copy link
Member

It is best not to do any migration, the simpler the better. We just need to make sure that both upgrades and downgrades are OK.

@Master92
Copy link
Contributor

It is best not to do any migration, the simpler the better. We just need to make sure that both upgrades and downgrades are OK.

Okay I respect your decision. But please add a remark to the next release that users have to re-configure that setting after an upgrade.

@ligenxxxx
Copy link
Member

Okay I respect your decision. But please add a remark to the next release that users have to re-configure that setting after an upgrade.

Understand, this setting becomes the default value after the upgrade.

@ligenxxxx ligenxxxx merged commit e0fe2b1 into hd-zero:main Nov 21, 2024
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.

3 participants