Skip to content

Removal of delay within USBDevice::ISRHandler #404

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
Merged
merged 2 commits into from
Apr 29, 2019
Merged

Removal of delay within USBDevice::ISRHandler #404

merged 2 commits into from
Apr 29, 2019

Conversation

aentinger
Copy link
Contributor

Previously a delayMicroseconds(20); was executed within USBDevice::ISRHandler in order to enable consistent triggering of bootloader upon firmware upload. This changes remove the need for the delay within the ISR.

@aentinger
Fixing typo - although control endpoints are located at endpoint #0 w…
f6ae6d8 
…e should leave this choice to the caller of the function and be consistent within the rest of the code
@aentinger
Deleting superflous call to 'usbd.epBank0ResetReady' - this function …
4c5ac51 
…is already called within 'USBDeviceClass::ISRHandler' and 'USBDeviceClass::recvControl' is called by 'handleClassInterfaceSetup' within the ISR

Bugfix - removal of delay within USBDeviceClass::ISRHandler
- remove stall requests of endpoint #0 OUT (bank #0) if present
- copy data from cache into structure instead of referencing it via pointer. When referencing via pointer the content of the structure can change if new data is written to the buffer.
- Fixing MULTI_PACKET_SIZE parameter of call to 'usbd.epBank0SetMultiPacketSize' within 'USBDeviceClass::armRecvCtrlOUT' to correct size, MULTI_PACKET_SIZE should not be set to a value < SIZE, this means at least to 64.
- resetting byte count for endpoint #0 seems to eliminate issue of the bootloader not being triggered.
@sandeepmistry
Copy link
Contributor

I've tested this on my Mac which runs macOS 10.14.4, with the following sketches:

  • VidorTestSketch
  • VidorDrawLogo
  • SerialPassthrough with a wire between the TX and RX pins on the header

All looks good so far, please let me know if there are additional sketches that I can run.

@aentinger
Copy link
Contributor Author

Thank you very much for trying out with those sketches. I've used VidorTestSketch and VidorDrawLogo myself as well as the generic Blink. I don't believe any additional sketches need to be run, but maybe it's beneficial to test "aggressively" by uploading one sketch and then immediately follow with uploading the other sketch, vary the delay between completion of upload and trigger of next upload, ... . So far I did not manage to produce a hang-up with those methods.

@aentinger
Copy link
Contributor Author

I've tried the patch successfully with VidorTestSketch and VidorDrawLogo on both Ubuntu 18.04 LTS and Windows 10.

@facchinm
Copy link
Member

Works perfectly, merging!

@facchinm facchinm merged commit 3c0fc11 into beta Apr 29, 2019
@aentinger aentinger deleted the cdc-bugfix branch April 29, 2019 11:03
@aentinger aentinger mentioned this pull request May 27, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sandeepmistry sandeepmistry Awaiting requested review from sandeepmistry

@facchinm facchinm Awaiting requested review from facchinm

@cmaglie cmaglie Awaiting requested review from cmaglie

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aentinger @sandeepmistry @facchinm