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.

Sign up for GitHub

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

Jump to bottom

Set outputs to off when saving settings #93

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Set outputs to off when saving settings #93

wants to merge 2 commits into from
+1 −0

Conversation

AlexKlimaj
Copy link
Contributor

@AlexKlimaj AlexKlimaj commented Sep 29, 2024 •
edited
Loading

Add an extra level of protection to turn off all the outputs before saving EEprom settings.

@AlexKlimaj AlexKlimaj changed the title Set outputs to off when saving settings Set outputs to off when saving settings. Remove unused saveEEpromSettings. Sep 29, 2024
@freasy
Copy link
Collaborator

freasy commented Sep 30, 2024

Thank you for your contribution.

Please explain the reason and the changes you made.

@AlexKlimaj AlexKlimaj force-pushed the pr-save-settings-outputs-off branch from 7add821 to 94884eb Compare September 30, 2024 15:52
@AlexKlimaj AlexKlimaj changed the title Set outputs to off when saving settings. Remove unused saveEEpromSettings. Set outputs to off when saving settings Sep 30, 2024
tridge
tridge approved these changes Oct 8, 2024
View reviewed changes
Copy link
Member

@tridge tridge left a comment

Choose a reason for hiding this comment

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

I think this is worthwhile to make sure we don't burn out a ESC by having a code path that does a flash write while running. A flash write involves a long delay with the CPU stalled (even interrupts can stall depending on flash layout)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tridge tridge tridge approved these changes

Assignees
No one assigned
Labels
dont merge improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AlexKlimaj @freasy @tridge