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

Sleep until USB port becomes writable before running avrdude #5393

Merged
merged 4 commits into from
Mar 13, 2019
Merged

Sleep until USB port becomes writable before running avrdude #5393

merged 4 commits into from
Mar 13, 2019

Conversation

datagrok
Copy link
Contributor

Description

Currently, running with :avrdude as a target will monitor /dev/tty* for the appearance of a new device, then sleep for one second, then launch avrdude to flash the keyboard.

For reasons that I haven't yet determined, that one-second sleep is insufficient on my system. /dev/ttyACM0 appears with root:root ownership, and it takes udev a good 2.5 seconds or so to change its permissions to root:dialout, giving my normal user write access. This causes avrdude (and make) to exit with an error most of the time:

avrdude: ser_open(): can't open device "/dev/ttyACM0": Permission denied

avrdude done.  Thank you.

make[1]: *** [tmk_core/avr.mk:228: avrdude] Error 1
Make finished with errors
make: *** [Makefile:551: mitosis:datagrok:avrdude] Error 1

To account for this I have replaced the 1-second sleep with a small loop, similar to the other, to wait for the device to become writable. This seems to successfully resolve the problem, at least on my Debian Linux machine.

If there is a known fix that will make my udev rules do their thing faster, I'm eager to hear about it.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

None that I am aware of.

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@drashna
Copy link
Member

drashna commented Mar 13, 2019

This breaks :avrdude on MSYS2 (Windows), as it never "becomes writable".

It looks like we need to check the OS before calling this routine.

using MINGW or MSYS: sleep for one second, as before.

otherwise: wait for the port to become writable.
@datagrok
Copy link
Contributor Author

@drashna I resurrected the sleep 1, moving it into the conditional that checks for MSYS, with my waiting-for-writable loop in its else block. Still accomplishes what I want on my system. Does this unbreak :avrdude on Windows? (I don't have a Windows box to test.)

@yanfali
Copy link
Contributor

yanfali commented Mar 13, 2019

needs testing on macOS too

Copy link
Contributor

@yanfali yanfali left a comment

Choose a reason for hiding this comment

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

verified on macOS

@drashna
Copy link
Member

drashna commented Mar 13, 2019

Confirmed working on MSYS2, now. Thanks!!

@drashna drashna merged commit f34244a into qmk:master Mar 13, 2019
@datagrok datagrok deleted the wait-for-port branch March 14, 2019 19:51
Shinichi-Ohki added a commit to Shinichi-Ohki/qmk_firmware that referenced this pull request Mar 15, 2019
* 'master' of https://github.com/qmk/qmk_firmware: (48 commits)
  [Keymap] Added Boy_314's layout for half n half keyboard (qmk#5373)
  Align use of atmega32a program script (qmk#5259)
  [Keyboard] new keyboard lovelive9 (qmk#5266)
  [Keyboard] Inital port of xd96 (qmk#5401)
  Fix ascii art (qmk#5407)
  [Keyboard] Georgi Support (qmk#5384)
  fresh commit for a new fork for PR to upstream/master (qmk#5406)
  Added info.json for mt980 keyboard and fixes to walker keymap (qmk#5391)
  Add support for THE60 (qmk#5385)
  Added 1up60rgb keymap: mdyevimnav (qmk#5386)
  Fix i2c calls for HotDox keyboard (qmk#5387)
  Sleep until USB port becomes writable before running avrdude (qmk#5393)
  [Keymap] Some more improvements to keymap, currency symbols.. (qmk#5395)
  [Keymap] Add atreus, ergotravel and org60 keymaps (qmk#5381)
  archetype keymap for jj50 (qmk#5397)
  Wheat Field Peripherals mt980 (FC980M Layout) PCB Support (qmk#5374)
  Minor readme fix (qmk#5389)
  Add new keyboard Plaid and ATMEGA328p support (qmk#5379)
  Next set of split_common changes (qmk#4974)
  [Keyboard] Lily58 Add info.json file (qmk#5354)
  ...
chie4hao pushed a commit to chie4hao/qmk_firmware that referenced this pull request Mar 28, 2019
* sleep until usb port becomes writable before running avrdude

* only wait for a writable USB port when not on MSYS

using MINGW or MSYS: sleep for one second, as before.

otherwise: wait for the port to become writable.

* typo

* typo
zer09 pushed a commit to zer09/qmk_firmware that referenced this pull request Mar 31, 2019
* sleep until usb port becomes writable before running avrdude

* only wait for a writable USB port when not on MSYS

using MINGW or MSYS: sleep for one second, as before.

otherwise: wait for the port to become writable.

* typo

* typo
danielo515 pushed a commit to danielo515/qmk_firmware that referenced this pull request May 15, 2019
* sleep until usb port becomes writable before running avrdude

* only wait for a writable USB port when not on MSYS

using MINGW or MSYS: sleep for one second, as before.

otherwise: wait for the port to become writable.

* typo

* typo
Timbus pushed a commit to Timbus/qmk_firmware that referenced this pull request Jun 23, 2019
* sleep until usb port becomes writable before running avrdude

* only wait for a writable USB port when not on MSYS

using MINGW or MSYS: sleep for one second, as before.

otherwise: wait for the port to become writable.

* typo

* typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants