Skip to content
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

removed some redundant lines #1494

Merged
merged 16 commits into from
Dec 13, 2022
Merged

removed some redundant lines #1494

merged 16 commits into from
Dec 13, 2022

Conversation

discip
Copy link
Collaborator

@discip discip commented Dec 7, 2022

as stated in the header
and corrected reset message

Reset message was in both cases the same, but the result was not!

@discip
Copy link
Collaborator Author

discip commented Dec 10, 2022

@Ralim
Some words regarding the 'reset-after-firmware-update' function.

Since I'm using a fork of IronOS without Invert & ButtonLocking, I noticed that the current handling of a firmware update in terms of a reset causes problems that make a manual reset unavoidable.

I assume that values ​​from previous functions are being moved and put in the place of other functions, resulting in a chaos.

In my case the device displayed a VIN of 8 V instead of 5 V!

So we either need to:

  1. rework the way a reset is performed, or
  2. reset all values to default after a firmware update.

I'd like to have the 1 option, but don't know how to achieve this. 😅

@Ralim
Copy link
Owner

Ralim commented Dec 10, 2022

Are you at all messing with the enumeration inside the settings file? That enumeration is append only (only ever add new ones) if any are reordered or removed you have to force a reset as that breaks the migration.

Additionally migration only works forwards, ie. On adding, if you go forwards to a new setting,back and then forwards again it may not behave well as it will shoehorn the former extended setting into the new one.

So long as that settings enumeration is not touched it should migrate fine.

@discip
Copy link
Collaborator Author

discip commented Dec 10, 2022

Sorry, but I am not able to follow. 😅
What do you mean by enumeration?

Additionally migration only works forwards, ie. On adding, if you go forwards to a new setting,back and then forwards again it may not behave well as it will shoehorn the former extended setting into the new one.

That is exactly what I did. 🤣

I primarily thought about a scenario where features might eventually be removed.

So long as that settings enumeration is not touched it should migrate fine.

So even the above mentioned case should work, right?

@discip
Copy link
Collaborator Author

discip commented Dec 10, 2022

I failed at making a return to the home screen after a factory reset! 😩
What am I missing? Is there an easy way of achieving that?

thanks in advance

@Ralim
Copy link
Owner

Ralim commented Dec 11, 2022

So this part of the code:

enum SettingsOptions {

Where it assigns a setting to a number; can never be changed since thats how they are laid out on flash.
So if we remove a setting its value will stay (and be lost) for now.

I failed at making a return to the home screen after a factory reset!
The best way to do this is just reboot the device tbqh to make it do a full settings reload.

@discip
Copy link
Collaborator Author

discip commented Dec 12, 2022

So this part of the code:

enum SettingsOptions {

Where it assigns a setting to a number; can never be changed since thats how they are laid out on flash. So if we remove a setting its value will stay (and be lost) for now.

Ok! Got that. 😃 👍🏻
Thank you!

The best way to do this is just reboot the device tbqh to make it do a full settings reload.

This seems a bit clumsy. I think it would be best if we could virtually reboot the device (without disconnecting it).
Is there a way to achieve this?

To clarify how I imagined it to work:
After the confirmation Reset OK appears, the device will play the bootup logo, followed by the home screen.

Nonetheless, I think this PR can be merged, as it re-establishes the separation between the two reset modes.
Unless you deem it unnecessary.

thanks in advance
kind regards

@discip discip mentioned this pull request Dec 13, 2022
@Ralim
Copy link
Owner

Ralim commented Dec 13, 2022

This seems a bit clumsy. I think it would be best if we could virtually reboot the device (without disconnecting it).
Is there a way to achieve this?

Sorry thats what I meant; we can just ask the chip to do a reset for us.
There is the command reboot(); but iirc it needs fixing on Pinecilv2. If ok; we can do that in new PR (want to kick 2.20 out)

@Ralim Ralim merged commit ebe08ed into Ralim:dev Dec 13, 2022
@discip
Copy link
Collaborator Author

discip commented Dec 13, 2022

Sorry thats what I meant; we can just ask the chip to do a reset for us.
There is the command reboot();

Ahh, that's fine! 👍🏻

but iirc it needs fixing on Pinecilv2. If ok; we can do that in new PR (want to kick 2.20 out)

I'm good with that!
Should I try to send a PR on the way (At least the part I think I can do.)?
Or should I leave it all up to you?

@discip discip deleted the patch-1 branch December 13, 2022 13:32
@discip
Copy link
Collaborator Author

discip commented Jan 9, 2023

@Ralim

Additionally migration only works forwards,

Since the feature removal method I used in my fork doesn't work, I would like to know how to do it properly?

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.

2 participants