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

Reboot after reset #1513

Merged
merged 10 commits into from
Jan 6, 2023
Merged

Reboot after reset #1513

merged 10 commits into from
Jan 6, 2023

Conversation

discip
Copy link
Collaborator

@discip discip commented Dec 30, 2022

This implements the reset behavior commonly used with other devices that reboot for the factory settings to take effect.

  • @Ralim fixing reboot(); for the Pinecil family

@discip
Copy link
Collaborator Author

discip commented Jan 3, 2023

@Ralim
First of all:

Happy new year!

Hope you are well. 😀

Any progress so far?

@Ralim
Copy link
Owner

Ralim commented Jan 3, 2023

Hia,
Happy new year too !!

I just pushed to Dev a potential fix, don't have hardware with me to test though

@discip
Copy link
Collaborator Author

discip commented Jan 3, 2023

Great! 😀
Will test as soon as possible.

@discip discip marked this pull request as ready for review January 3, 2023 14:57
@discip
Copy link
Collaborator Author

discip commented Jan 3, 2023

Works for both TS80P and Pinecil.

@river-b
Maybe you could check if performing a reset on the Pinecil V2 does reboot it after flashing this build.

@Ralim
Otherwise, this can be merged.

@discip
Copy link
Collaborator Author

discip commented Jan 4, 2023

@Ralim
I think this can be merged as all other devices (MHP30, TS100) also need to be tested.
And waiting for someone to test this isn't as efficient as doing it the other way around, because if this will cause any issues, it will be reported straight away. 😁

@Ralim Ralim merged commit 7f43562 into Ralim:dev Jan 6, 2023
@discip discip deleted the reboot_after_reset branch January 6, 2023 07:34
@Ralim Ralim added this to the 2.21 milestone Jan 17, 2023
@River-Mochi
Copy link
Contributor

River-Mochi commented Feb 17, 2023

@discip
Tested Pinecil V2 with one of the beta 2.20.FE159BA dated Feb 16, 2023 (although it seems like this would be a 2.21 beta and not 2.20 but that is what was on the screen).

  1. Advanced menu, Restore default settings
  2. pressed (+) 1-2 times, saw message "Reset OK"
  3. then the screen flickered and went back to main screen icons
  4. I checked and my custom settings were gone so it did sucessfully reset to defaults (detailed idle screen was back to unchecked).
    Suggestion
    Consider changing the success message to "Complete!" or "Reset done!" instead of "Reset Ok". I was not initially sure if "reset ok" meant it was done. Something stronger would be helpful to reassure people they did it correctly.

Side note for possible user error that might come up:
Since I was trying things to see what users might do by accident and how to possibly break this based on normal user behavior, I did the reset a few times different ways, sometimes I would press the (+) button 2-3 times quickly or hold the button a little to see what would happen.
There is a behavior that might happen to some people: if I press the (+) button too many times quickly to confirm the reset, for example I once pressed it 2-3 times in a row, I got the PD Debug menu showing State 6. This is normal I think because normally if the (+) button is held down when plugging in the cable while powering up, then you enter the PD Debug menu (but in a normal situation it would show State 12).
And if this Reset is rebooting the pinecil and I happen to be pressing the (+) button too many times and holding it down at the right moment, then instead of going to the main screen, it will display PD Debug menu with State 6.
If this issue comes up in an Issue ticket, we simply have to advise people to not hold down the (+) button to click it too many times to Reset. Once they get the confirmation message on the screen, then just wait 1 second, and it will go back to the main screen icons. Pressing or holding the (+) too many times while attempting the reset will enter PD debug menu, I was able to reproduce this a few times.
The issue is easily resolved if one is in PD state 6 screen, simply unplugging and plugging back in fixes it.

@discip
Copy link
Collaborator Author

discip commented Feb 25, 2023

@river-b

Consider changing the success message to "Complete!" or "Reset done!" instead of "Reset Ok". I was not initially sure if "reset ok" meant it was done. Something stronger would be helpful to reassure people they did it correctly.

I think the initial thought behind using Reset OK was to make it unmissable by using a large font. And since this message doesn't scroll, only that phrase will fit on the screen.

There is a behavior that might happen to some people: ...

A user who manages to do this would be a very impatient person so it is generally not advisable to let them have a soldering iron as they would end up burning things and in the worst case even hurt themselves. 🤷🏻

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