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

USB-Core Fixes #3562

Closed
wants to merge 4 commits into from
Closed

USB-Core Fixes #3562

wants to merge 4 commits into from

Conversation

NicoHood
Copy link
Contributor

Backported a few fixes from https://github.com/NicoHood/HID

  • Flexible, global Endpoint size (USB_EP_SIZE)
  • Added Weak CDC LineEncoding functions
  • Added CDC LineEncoding information functions
  • Fixed Magic Key for newer boards
  • Added some useful definitions
  • Minor fixes
  • Added u2 Series support

Keywords for the Serial functions should be added as well. Inlining those functions doesnt work since its a static variable. I think this is the most save and easy way to access the data for advanced users.

Should we change the Bootkey (for the non 32u4 boards) to a 1byte value? Thats normally enough and saves flash in the bootloader and sketch itself. I'd go for it, since I have to change my HoodLoader2 anyways.

NicoHood added a commit to NicoHood/HoodLoader2 that referenced this pull request Jul 22, 2015
This way a key can be specified in the pins_arduino.h file
@matthijskooijman
Copy link
Collaborator

IMHO, a single big commit that just says "Applied fixes from HID-Project development" messes up the history badly, those changes should at least be documented in detail, but preferably be split into multiple commits. If you already have them as separate commits in your HID-project repository, you can probably fiddle a bit with git format-patch and git am to port over the separate commits.

Regarding the baud(), dtr(), etc. accessors, I also added these in #3343, so it might be better to drop those. Regarding the weak event callbacks, also see the discussion in #3343 where some objections to such a weak API are made. What is the actual usecase you need these week APIs for?

@NicoHood
Copy link
Contributor Author

NicoHood commented Aug 5, 2015

I am not that git pro. But I can make a new PR if wanted.

The Events were made for this usage:
https://github.com/NicoHood/HID/blob/master/examples/Projects/USB-Serial/USB-Serial.ino

@matthijskooijman
Copy link
Collaborator

@NicoHood You should be able to implement the same sketch by simply polling the dtr() and baud() methods, see my PR for an example sketch that I believe essentially does the same (and even adds break support).

@NicoHood
Copy link
Contributor Author

NicoHood commented Aug 6, 2015

Could be possible, have to check that.

Wouldnt it also make sense to mark the USB functions as weak, so one can overwrite the USB clock settings etc? Then other MCUs can be supported with a library via a strong implementation which overwrites those functions. Or even with a .cpp file in the pins_arduino.h

This would help if the Arduino team does not want to include my u2 support code to the core itself (if there is any reason for this, dunno).
USB_ClockEnable() and Disable()

@NicoHood
Copy link
Contributor Author

NicoHood commented Aug 7, 2015

Should I open a new PR with this branch?
https://github.com/arduino/Arduino/compare/master...NicoHood:USB-Core-Fixes?expand=1

I had so many problems to get my git in sync with the master, I am not able to repair this PR, so I'd create a new one if desired. This patch has nothing complicated in it and the commits are now more clean. Please have a look.

@NicoHood NicoHood mentioned this pull request Aug 8, 2015
@NicoHood
Copy link
Contributor Author

NicoHood commented Aug 8, 2015

Moved to #3640

@NicoHood NicoHood closed this Aug 8, 2015
@ffissore ffissore modified the milestone: Release 1.6.6 Aug 11, 2015
@NicoHood NicoHood deleted the USB-Fix branch September 28, 2015 15:09
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.

4 participants