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

[1.6.6 Pluggable HID] Compiling issues for custom HID libraries #3649

Closed
NicoHood opened this issue Aug 9, 2015 · 6 comments
Closed

[1.6.6 Pluggable HID] Compiling issues for custom HID libraries #3649

NicoHood opened this issue Aug 9, 2015 · 6 comments
Assignees
Labels
Component: Core Related to the code for the standard Arduino API
Milestone

Comments

@NicoHood
Copy link
Contributor

NicoHood commented Aug 9, 2015

This PR added Pluggable HID with IDE 1.6.6
#3304

I started developing some custom HID devices.
See this commit as reference: https://github.com/NicoHood/HID/tree/4df1f30427730c43216336b94673f66b546fc6d4/src

It turns out that because of static linkage all files are compiled. This means Mouse, Gamepad, System and Consumer files are all compiled and linked. Now try to open the Mouse example. It wont work because the singletons (Consumer, Gampad, System instances) add their descriptor in the constructor. I dont know why this still happens, even if you do not include the other files they will cause trouble.

First I did not notice this, because not all HID devices cause problems when used together. The mouse button was working but the moving did not. The problem is a mix of incompatible USB devices, cause by accidentialy compiled files.

So I am not sure if this is expected to be this way, but this feature request could possibly help us:
#2800

The solution for now is to delete the other .h/cpp files (if you want to try it yourself with the commit above). The long time solution would be to create 4 different libraries. This is bad for the boards manager. It'd be better if you can add a whole HID package instead of 4 (and later even more).

@facchinm

@facchinm
Copy link
Member

Hi Nico, the library compilation method (all .cpp files get compiled) plus the linker strategy (all .o file become part of the .a) plus the pluggable method (using singletons) do, in fact, cause your issue.
I can see two solutions:

1 - if you drop your cpp files (which are almost all empty), declare the singleton in the .h without extern and include only what you need, no extra descriptor should be included

2 - standalone libraries; I can't see the problem with the library manager, quite the opposite in fact. From the dev point of view, you could have a development repo with all the libraries together and some handy scripts using git-filterbranch that, once you want to release, will take care to extract the history of every standalone library, tag it and push it

3 - (this in not a real solution 😄 ) you can also drop the pluggable-HID feature and base everything on pluggable-USB (and give your users infos on how to enable/disable the library features), somehow like CompleteHID library. In this way you'll have full control over what is included and what is not, you can handle the coexistence between different functions (which you can't do with the standalone libraries) and also reduce the memory footprint.

@facchinm facchinm added Component: Core Related to the code for the standard Arduino API Waiting for feedback More information must be provided before we can proceed labels Aug 10, 2015
@NicoHood
Copy link
Contributor Author

With separate .a linkage (see issue feature request) this wont happen I think. That how the IDE files get compiled.

1.) This causes problems if you want to call the HID stuff within another library. Really bad idea. I also have to rename the HID Descriptor name in the .h file or multiple libs will cause errors.

2.) is it possible for the boards manager to support different branches on a single repository? I dont know about those scripts, how could I do this? That'd be an idea at least

3.) nah, but thx :D

@matthijskooijman
Copy link
Collaborator

  1. seems too fragile to me, AFAICS that relies on including such a .h file from at most one source file (normally the .ino), which could easily break if other libraries get involved.

  2. could work, putting things in separate libraries. Not sure how to best manage this git- and library-manager-wise, though.

Implementing #2800 would be the most elegant solution, I guess.

@NicoHood
Copy link
Contributor Author

So is there a chance to see #2800 implemented? Otherwise I dont know how to continue with the project. Developing will be a mess. I also dont know how such a git script would look like but I think its still not a good solution. And #2800 could also help improving the PinChangeInterrupt and other libraries as well.

@NicoHood
Copy link
Contributor Author

This issue is solved with this PR:
#3697

I've tested it with my PinChangeInterrupt library and the HID-Project dev branch.
It would be nice if you can also test this to make someone merge the PR as fast as possible till the next release, when the new USB-Core will be released.

@ffissore
Copy link
Contributor

Issue moved to arduino/arduino-builder#10

@ffissore ffissore added this to the Release 1.6.6 milestone Sep 23, 2015
@ffissore ffissore removed the Waiting for feedback More information must be provided before we can proceed label Sep 23, 2015
@ffissore ffissore assigned ffissore and unassigned cmaglie Sep 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Related to the code for the standard Arduino API
Projects
None yet
Development

No branches or pull requests

5 participants