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

Add capability to force stay in DFU (and erase app) with button if #294

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ccrome
Copy link
Contributor

@ccrome ccrome commented Mar 1, 2023

Checklist

  • Please provide specific title of the PR describing the change
  • Please provide related links (eg. Issue which will be closed by this Pull Request)

Description of Change

Add capability to force stay in DFU (and erase app) with button if TINYUF2_DFU_BUTTON is set to 1.

This adds a check to see if a button is pressed. If the board button is pressed, stay if dfu mode.

If TINYUF2_DFU_BUTTON_ERASE is also set to 1, then the app is erased.

Initially opened discussion here. But decided to just go ahead and open a PR. Feel free to close this if you think it's not a feature you want to have integrated.

TINYUF2_DFU_BUTTON is set to 1.

This adds a check to see if a button is pressed.  If the board button is
pressed, stay if dfu mode.

If TINYUF2_DFU_BUTTON_ERASE is also set to 1, then the app is erased.
Copy link
Member

@hathach hathach left a comment

Choose a reason for hiding this comment

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

indeed, we have some thought to support DFU button, but haven't put much effort since double tap serve us well. Regarding erasing app, please check out the review for detail.

src/board_api.h Show resolved Hide resolved
src/main.c Show resolved Hide resolved
#ifndef TINYUF2_DFU_BUTTON
#define TINYUF2_DFU_BUTTON 0
// Should holding the DFU button perform an erase as well?
# ifndef TINYUF2_DFU_BUTTON_ERASE
Copy link
Member

Choose a reason for hiding this comment

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

this is a good idea, though we should only erase app if button is hold long enough e.g 5 seconds, there would be an sequence of indicator e.g

  • within second 0-4 the led will start to blink slowly, and
  • at 5 second it will either blink faster or solid light. Also change RGB color as well.
  • Only start erasing if user left off the button to prevent re-enter this mode once erasing complete

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm thinking about how to implement this, and I'm not 100% sure.

The system timer ticks fire at different intervals depending on what state things are in. Does it make sense to have a 1ms tick, and base everything off of that? That would make timing specific times like 5 seconds for erase a little more straightforward.

The alternative is to keep track of the current tick interval, and use that to count. But that will fail if the ticks are turned off during any time you want to keep track of time intervals.

Copy link
Member

Choose a reason for hiding this comment

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

I would add a new indicator state for holding button, that would make thing in sync with other. The timer interval could be choose to be a bit different than other state (along with RGB) mostly to inform user.

Copy link
Contributor Author

@ccrome ccrome Mar 9, 2023

Choose a reason for hiding this comment

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

I'm actually working on 2 separate projects: 1 deeply embedded with no access to buttons, and one handheld with too-easy access to buttons. The latter case has exactly 2 buttons total, one of which is used for the DFU mode as well as a standard meaning once the app starts.

Here's the scenario:

  • The user powers up the device, and is accidentally pressing the DFU button, because that button happens to have function during normal device operation.
  • The device is battery powered, so there's not a trivial way to completely remove power.
  • The user doesn't know anything about DFU, upgrades, or anything else. He just wants his device to work.
  • If there was an immediate erase, that would be bad -- the user would have a device stuck in DFU mode, nowhere near a computer, nor any idea how to recover it.
  • The device will stay in DFU mode forever unless we put some other timeout in there.

So, here's my proposed behavior that I think works for everything:

  • If the button is pressed at check_dfu_mode(), then stay in DFU mode
  • If the button is released within the timeout, then boot the app if it exists.
  • If the button is held longer than 5 seconds, erase the app.

Is there really any point in staying in the DFU and not erasing the app? I can't really imagine why you'd want to force an 'enter DFU state' but not erase the app.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just pushed a version with my proposed behavior. It's a bit more of a change than I thought it would be -- but I'm not sure I could make it a whole lot simpler -- just the way all the bits interact seem to reach into a few more places than expected.

I also updated the LED specifications so the timer interval is specified in the same spot as the RGB colors. I hope that's okay. If not, I can revert it back.

@ccrome ccrome force-pushed the button-stay-in-dfu branch 8 times, most recently from dd9b04e to e8f3249 Compare March 12, 2023 02:11
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