Skip to content

Add esp32s2 safe mode support & fix user_safe_mode output #3395

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 6 commits into from
Sep 13, 2020
Merged

Add esp32s2 safe mode support & fix user_safe_mode output #3395

merged 6 commits into from
Sep 13, 2020

Conversation

microdev1
Copy link
Collaborator

Please go through issue #3389 for a background of this PR.

What works:

  • On esp32s2, one can manually enter safe mode by pressing boot button during the 700ms loop on startup. This loop is indicated by:
    • yellow color on rgb led and/or,
    • a blinking led
      Note: If you do not have any way of guessing the loop status, power-on the board/press the reset button and after a short delay, press and hold boot button for a while
  • This PR also fixes the output of USER_SAFE_MODE which provides a board specific reason for entering safe mode.

To-do:

As of now when boot button is pressed esp32s2 doesn't perform a soft_reboot with safe_mode data stored in ram, instead it continues to boot with safe_mode status being USER_SAFE_MODE and finally ends up in safe mode. This workflow is not consistent with other CPY ports where a reset is performed and safe mode is entered on next boot.

To address this the following needs to be implemented:

  1. Instead of returning USER_SAFE_MODE in function wait_for_safe_mode_reset(), soft_reboot needs to be called.
  2. port_set_saved_word() function needs to edited in order to save safe_mode status in ram for the next boot.

tannewt
tannewt previously approved these changes Sep 11, 2020
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

This looks great! This essentially does match behavior of other ports already and this doesn't need to change.

We will need the soft reboot approach for other safe mode failures though.

@microdev1
Copy link
Collaborator Author

Yes, for other safe mode failures soft reboot approach is needed.
Also, I get weird text formatting issues when using screen utility as my serial console.

serial console error

@microdev1 microdev1 marked this pull request as ready for review September 12, 2020 13:28
@microdev1
Copy link
Collaborator Author

Current Status:

  • soft reboot approach is not yet implemented,
    some core functions need to be updated before it can be properly implemented.

What changed:
(since last review)

  • A fall-through reason was added in case BOARD_USER_SAFE_MODE_ACTION is not defined in the board specific config file.
  • There was a text formatting issue which I traced back to serial_write("\n") , it advances a line but doesn't set the cursor to the beginning of the next line. Fixed.
    This issue was only evident in UNIX based serial console environment.

@microdev1 microdev1 requested a review from tannewt September 12, 2020 13:54
Copy link

@jepler jepler left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! I have some comments that probably stem from me suggesting incorrect code on Discord, I've tried to correct it and explain my logic.

@microdev1 microdev1 requested a review from jepler September 13, 2020 18:30
Copy link

@jepler jepler left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes I requested!

@jepler jepler merged commit 73ad78e into adafruit:main Sep 13, 2020
@microdev1 microdev1 deleted the safeMode branch September 18, 2020 12:42
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