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

Optional .a linkage for libraries #3697

Closed
wants to merge 2 commits into from
Closed

Conversation

NicoHood
Copy link
Contributor

Fixes:
#2800
#3649
And adds the possibility to create more enhanced libraries with smaller flash size. This feature is optional and won't break any existing library.

All you need to do is add a alinkage=true to your library.properties file. By default alinkage is off, to not break older libraries with ISRs. A linkage is only possible for newer libraries with a /src folder.

Most of the code was adapted from here, so it should work fine, I did not reinvent the wheel here.
https://github.com/arduino/Arduino/blob/master/arduino-core/src/processing/app/debug/Compiler.java#L1130-L1182

The code now moved in its own function (2nd commit) to reduce code duplication.

I am still not sure if we need recursiveCompileFilesInFolder() or if we can use compileFiles() with true as paramter. If desired I can add a new recursiveCompileFilesInFolder() which returns a List of Files instead of void, if that helps in any case. What was the thought behind this? For this PR it is not critical since .a linkage is optional and won't break anything, and the core compiling stays the same.

Less code duplication
@matthijskooijman
Copy link
Collaborator

@ArduinoBot build this please.

@NicoHood
Copy link
Contributor Author

No you need to write @ArduinoBot, Build this please without the dot :D

@cmaglie cmaglie added Component: IDE The Arduino IDE Component: Compilation Related to compilation of Arduino sketches feature request A request to make an enhancement (not a bug fix) labels Aug 31, 2015
@cmaglie
Copy link
Member

cmaglie commented Sep 1, 2015

@ArduinoBot build this please

@cmaglie
Copy link
Member

cmaglie commented Sep 1, 2015

BTW the proposed patch looks good to me (y).

@NicoHood
Copy link
Contributor Author

NicoHood commented Sep 1, 2015

The easiest thing would be to merge it ;)
I've been using this for quite a while now and it works perfectly with and without .a linkage.

@matthijskooijman
Copy link
Collaborator

I haven't gotten around to looking at the patch in detail yet, should be able to do that in 1, max 2 weeks from now.

@matthijskooijman
Copy link
Collaborator

Seems this patch should be re-done for the new arduino-builder, now. @ffissore, any plans to include this? @NicoHood, care to learn the Go language too? :-)

@NicoHood
Copy link
Contributor Author

If you tell me the files I need to have a look at after the change :S I dont care if its go, java or c.

But it should be easy for @ffissore to include this since he just wrote the compiler. So he knows where to search and how to add it best.

This seems promising:
https://github.com/arduino/arduino-builder/blob/master/src/arduino.cc/builder/libraries_loader.go
https://github.com/arduino/arduino-builder/tree/master/src/arduino.cc/builder/phases
https://github.com/arduino/arduino-builder/blob/master/src/arduino.cc/builder/builder_utils/utils.go

@NicoHood
Copy link
Contributor Author

I've looked through the sources now and beside I do not fully get the Go language there is not a single commentary. So I am pretty much helpless here. I linked the useful files above but I am currently unable to understand how the .a linkage was used here. I also dont know how I should compile the files (the go files) anyways.

A bit feedback from @ffissore would be appreciated. Thanks.

@ffissore
Copy link
Contributor

Closing as I'm working on it. Tracking issue is arduino/arduino-builder#10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Compilation Related to compilation of Arduino sketches Component: IDE The Arduino IDE feature request A request to make an enhancement (not a bug fix)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants