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

check every 100ms for DFU mode #2836

Merged
merged 1 commit into from
Feb 28, 2022
Merged

check every 100ms for DFU mode #2836

merged 1 commit into from
Feb 28, 2022

Conversation

ctzsnooze
Copy link
Member

@ctzsnooze ctzsnooze commented Feb 27, 2022 •
edited
Loading

Rather than wait a full 5s to flash on my Mac, this code checks for a DFU connection every 100ms, then flashes when DFU is available.

Huge THANK YOU to @haslinghuis for re-factoring my original code so well, and for @limon for showing me the basics of how to do it! Very grateful for your help.

On my mac, most DFU connections take around 1.6 - 1.8s. Keen to hear how long it takes on Linux / Windows.

Typical console log at connect time.:

serial: closed connection with ID: 1, Sent: 19 bytes, Received: 77 bytes
port_handler.js:129 PortHandler - Removed: [{"path":"/dev/cu.usbmodem0x80000001","displayName":"Betaflight STM32F411","vendorId":1155,"productId":22336},{"path":"/dev/tty.usbmodem0x80000001","displayName":"Betaflight STM32F411","vendorId":1155,"productId":22336}]
stm32.js:121 DFU available after 1.8 seconds
stm32usbdfu.js:98 USB DFU detected with ID: 0
stm32usbdfu.js:124 Device opened with Handle ID: 1

We could check only every second, I guess, but it's interesting to see how quickly different systems connect.

@github-actions

This comment has been minimized.

Sign in to view
@ctzsnooze ctzsnooze added this to the 10.8.0 milestone Feb 27, 2022
@github-actions

This comment has been minimized.

Sign in to view
@haslinghuis haslinghuis force-pushed the check-every-100ms-for-dfu-mode-when-flashing branch from 513e812 to f4031ab Compare February 27, 2022 19:57
@haslinghuis haslinghuis added this to For discussion in Finalizing Firmware 4.3 Release via automation Feb 27, 2022
@haslinghuis haslinghuis moved this from For discussion to Configurator in Finalizing Firmware 4.3 Release Feb 27, 2022
haslinghuis
haslinghuis previously approved these changes Feb 27, 2022
View reviewed changes
@github-actions

This comment has been minimized.

Sign in to view
@ctzsnooze ctzsnooze force-pushed the check-every-100ms-for-dfu-mode-when-flashing branch from f4031ab to c9338fd Compare February 27, 2022 20:58
@ctzsnooze
Copy link
Member Author

Thanks Mark @haslinghuis !

That's a much nicer way to code it. Really grateful.

Works great on all boards I've tried so far (F722, F405, F411).

@ctzsnooze ctzsnooze self-assigned this Feb 27, 2022
haslinghuis
haslinghuis previously approved these changes Feb 27, 2022
View reviewed changes
@github-actions

This comment has been minimized.

Sign in to view
asizon
asizon requested changes Feb 27, 2022
View reviewed changes
src/js/protocols/stm32.js
}
}

let failedAttempts = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Why define after use it into the function?

Copy link
Member

@haslinghuis haslinghuis Feb 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The function is called in the setInterval below. The function definition is above the call. No need for hoisting. Parameters are still initialized before calling the function.

src/js/protocols/stm32.js Outdated
}

let failedAttempts = 0;
const dfuWaitInterval = setInterval(waitForDFU, 100);
Copy link
Member

Choose a reason for hiding this comment

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

Same here

Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

@blckmn
Copy link
Member

blckmn commented Feb 28, 2022

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> FAIL
  • assigned to a milestone -> PASS
  • cooling off period lapsed -> PASS
  • commit count less or equal to three -> PASS
  • Don't merge label NOT found -> PASS
  • at least one RN: label found -> PASS
  • Tested label found -> PASS
  • assigned to an approver -> FAIL
  • approver count at least three -> FAIL

limonspb
limonspb previously approved these changes Feb 28, 2022
View reviewed changes
Copy link
Member

@limonspb limonspb left a comment

Choose a reason for hiding this comment

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

nice!
looks good, but if there will be some change requests, plz rename
waitForDFU -> waitForDfu

@ctzsnooze ctzsnooze dismissed stale reviews from limonspb and haslinghuis via 249345b February 28, 2022 06:42
@ctzsnooze ctzsnooze force-pushed the check-every-100ms-for-dfu-mode-when-flashing branch from c9338fd to 249345b Compare February 28, 2022 06:42
@sonarcloud
Copy link

sonarcloud bot commented Feb 28, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-actions
Copy link
Contributor

Do you want to test this code? Here you have an automated build:
Betaflight-Configurator-Debug-Android
Betaflight-Configurator-Debug-Linux
Betaflight-Configurator-Debug-macOS
Betaflight-Configurator-Debug-Windows
WARNING: It may be unstable and result in corrupted configurations or data loss. Use only for testing!

@limonspb limonspb requested a review from asizon February 28, 2022 09:11
Finalizing Firmware 4.3 Release automation moved this from Configurator to Firmware Feb 28, 2022
@haslinghuis haslinghuis merged commit a8a9ac6 into betaflight:master Feb 28, 2022
Finalizing Firmware 4.3 Release automation moved this from Firmware to Finished (Merged) Feb 28, 2022
@haslinghuis haslinghuis mentioned this pull request Mar 31, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@limonspb limonspb limonspb approved these changes

@haslinghuis haslinghuis haslinghuis approved these changes

@asizon asizon asizon approved these changes

Assignees

@ctzsnooze ctzsnooze

Labels
RN: IMPROVEMENT Tested
Projects
No open projects
Finalizing Firmware 4.3 Release
Finished (Merged)
Milestone
10.8.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ctzsnooze @blckmn @limonspb @haslinghuis @asizon