Skip to content

Add automatic lib detection #341

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

Closed
wants to merge 1 commit into from
Closed

Add automatic lib detection #341

wants to merge 1 commit into from

Conversation

ladislas
Copy link
Contributor

@ladislas ladislas commented Apr 5, 2015

Hey there!

It's been a while, but I'm back with my automatic library detection PR ;)

I've tested it on Bare Arduino Project and Moti and it works great.

Would love to have some feedbacks :)

@sej7278
Copy link
Collaborator

sej7278 commented Apr 5, 2015

the main thing that bugs me is shelling out to another python script :-(

what does this do that the current detection doesn't - libs that call other libs or something? as system, platform and user lib autodetection already works quite well i think.

can you provide some build output so we can get an idea?

@ladislas
Copy link
Contributor Author

ladislas commented Apr 5, 2015

libs that call other libs or something?

Yes exactly. The idea is to be able to only include the main lib in your sketch and let the makefile figure out which other libs it needs to link and compile.

This is something the IDE is going to support real soon.

Here is the output of the build process: https://gist.github.com/ladislas/b5d245e9f8f4ffa6a26a

@sej7278
Copy link
Collaborator

sej7278 commented Apr 5, 2015

i'm surprised it doesn't remove a shedload of code from Arduino.mk then, for instance all of this lot becomes redundant doesn't it?

https://github.com/sudar/Arduino-Makefile/blob/master/Arduino.mk#L825
https://github.com/sudar/Arduino-Makefile/blob/master/Arduino.mk#L916
https://github.com/sudar/Arduino-Makefile/blob/master/Arduino.mk#L1154

another two things i can think of - we'd need to search for the binary in the path, not just ARDMK_DIR/bin, like we do here (as debian etc. would want it in /usr/bin/):

https://github.com/sudar/Arduino-Makefile/blob/master/Arduino.mk#L737

which probably means giving it a more specific name than "lib-detection", maybe "ard-lib-detection"?

and also it would need a manpage, which also means we'd have to update the fedora specfile (and the debian package).

and replace tabs with spaces in the python script.

and the regex only matches quotes not brackets, so this doesn't work: #include <mylib.h>

edit: hang on, does this only work on user libs?

this regex works for me:

includeRegex = re.compile("(?<=^\#include\s\")(.*)(?=\.h\")", re.DOTALL|re.M)

then use group = match.group(2)

although there's a stray newline somewhere between system/platform:

- [AUTODETECTED]       Size utility: AVR-aware for enhanced output
-
-                      SKETCH_LIBS =
- [USER]                 DallasTemperature
- [USER]                 OneWire
- [USER]                 RF24
- [USER]                 RF24Network
-
-                      SYSTEM_LIBS =
- [SYSTEM]               Servo
-
- [PLATFORM]             SPI

Here's my "fixes" for the above issues for you to look at (minus the manpage):
https://github.com/sej7278/Arduino-Makefile/tree/auto-lib

but it still seems to be replacing code without actually removing it from Arduino.mk (maybe?) and i'm still not convinced, it seems to be replacing USER_LIBS and ignoring ARDUINO_LIBS if manually set. and it doesn't find libs in the same directory as the sketch as it only searches USER_LIB_PATH, so doesn't appear to be any better than the current auto-detection.

Does this fix any of these issues?

#93
#251
#249

Either way, we should probably tag a new release before merging this sort of big change in.

sej7278 added a commit to sej7278/Arduino-Makefile that referenced this pull request Apr 5, 2015
sej7278 added a commit to sej7278/Arduino-Makefile that referenced this pull request Apr 6, 2015
@ladislas
Copy link
Contributor Author

ladislas commented Apr 6, 2015

@sej7278

thanks for all your feedbacks! :) I'm currently working on it and to answer your questions:

does it still output the object files to different directories like userlibs, platformlibs, libs, core etc?

yes.

we'd need to search for the binary in the path, not just ARDMK_DIR/bin

Ok, I'll do that.

which probably means giving it a more specific name than "lib-detection", maybe "ard-lib-detection"?

Changing that.

and replace tabs with spaces in the python script.

sure.

and the regex only matches quotes not brackets, so this doesn't work: #include <mylib.h> -- hang on, does this only work on user libs?

yes, the idea was to only work on user libs and not on the Arduino core and platform libs. Do you think it could be useful as well?

it seems to be replacing USER_LIBS and ignoring ARDUINO_LIBS if manually set.

I'll look into that as well as into the useless code.

Can we close PR #291 ?

Already done.

Does this fix any of these issues?

Yes for #93 and #251. Haven't tried #249 yet.

Either way, we should probably tag a new release before merging this sort of big change in.

Agreed

@sej7278
Copy link
Collaborator

sej7278 commented Apr 6, 2015

@ladislas - if you look at my branch linked above, i've already fixed most of the items other than removing "legacy" code and writing a manpage. see what you think.

i don't think its worth supporting platform/system libs, it'll just slow things down more and they shouldn't change often anyway. might be worth documenting though.

issues #249 / #251 don't seem to be fixed for me. libraries stored within my sketch directory are not printed, although they seem to be linked:

directory:

wiichuck/
├── Makefile
├── pindefs.h
├── WiiChuck
│   └── WiiChuck.h
└── wiichuck.ino

output:

-                      ARDUINO_LIBS =
- [SYSTEM]               Servo
- [PLATFORM]             Wire

sketch:

#include <Wire.h>
#include <Servo.h> 
#include "WiiChuck/WiiChuck.h"
#include "pindefs.h"

Although I guess putting libraries inside the sketch is bad form anyway, and the IDE wouldn't work with that either.

sej7278 added a commit to sej7278/Arduino-Makefile that referenced this pull request Apr 7, 2015
sej7278 added a commit to sej7278/Arduino-Makefile that referenced this pull request Apr 8, 2015
@ladislas ladislas closed this Sep 25, 2016
@ladislas ladislas deleted the auto-lib branch September 25, 2016 10:48
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.

2 participants