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

Feature/vendorlibraries #1009

Merged
merged 7 commits into from
Jun 15, 2016
Merged

Feature/vendorlibraries #1009

merged 7 commits into from
Jun 15, 2016

Conversation

m-mcgowan
Copy link
Contributor

@m-mcgowan m-mcgowan commented May 25, 2016

Adds an APPLIBS variable that defines the location of 'tweaked' particle libraries.

To test,

  • download the freertso4core library
  • create a new directory /usr/local/myapp for an application, separate from the library directory, and copy thetimer.cpp example from the library to that directory.
  • perform the library tweaks as described in the build documentation (in this PR)
  • build the example file with the app by running
make APPDIR=/usr/local/myapp APPLIBS=/path/to/freertos4core

The build should succeed.


Doneness:

  • Contributor has signed CLA
  • Problem and Solution clearly stated
  • Code peer reviewed
  • API tests compiled
  • Run unit/integration/application tests on device
  • Add documentation
  • Add to CHANGELOG.md after merging (add links to docs and issues)

…ke recipes for the library directory, so that the make system knows how to compile a file in the library directory and produce an object file in the build target directory. Scripts generating scripts...time to find a more expressive tool. Oh hello CMake!
@m-mcgowan m-mcgowan added this to the 0.6.x milestone May 25, 2016
@suda
Copy link
Member

suda commented May 27, 2016

I think the APPLIBS variable only takes one directory but it should allow passing multiple ones separated by ;. Rationale here is to pass exact path to libraries instead of directory that contains libraries to be able to select version i.e.:
APPLIBS=~/particlelibs/neopixel@0.1.0;~/particlelibs/internetbutton@0.2.0
and still be able to include them in Arduino compatible way:

#include <neopixel.h>
#include <internetbutton.h>

@m-mcgowan
Copy link
Contributor Author

Ok, I may have confused our stop-gap support for libraries v1 with our efforts on libraries v2. What I coded here allows multiple directories containing v1 libraries to be specified. (I coded this last sprint, which was all about v1 library support, which was the fuel for my confusion! ;-) )

Do we even need v1 external vendored libraries? I don't believe so but would like confirmation. If not, then I'll change this to take a list of absolute library directories, as per the v2 spec.

@m-mcgowan m-mcgowan self-assigned this Jun 8, 2016
@suda
Copy link
Member

suda commented Jun 8, 2016

We only need this for v2. I wanted this to be in 0.6.0 so the feature would be in the wild already when we release libs v2.

…PPLIBSV2. V2 libraries build sources under the src/ folder under each library.
@m-mcgowan
Copy link
Contributor Author

testing (requires bats to be installed)

cd build/test
bats libs.bats

@mrmowgli
Copy link

+1 on the multiple library dirs.

Sorry just jumping in because I wanted to clarify, this is currently a v1 version of this, but this is going to be refactored as a v2 feature?

How are external libraries handled now?

@m-mcgowan
Copy link
Contributor Author

Hi @mrmowgli!

Libraries in firmware are not presently handled with the local build. Before this PR, the only option was to manually copy them to your source folder to create a structure like this:

 - app/
 +- myapp.cpp
 +- lib1/
 +-- lib1/lib1.h
 +- lib2/
 +-- lib2/lib2.h

With APPLIBSV1 defined to a directory containing the libraries you want to use, you can place the libs outside of your application source, so less copying of files.
The APPLIBSV2 variable is similar, but uses the next revision of the libraries. More details on that later. We are working hard on making libraries supported equally across all our tools, and using a new format that is compatible with Arduino, so 3rd party libraries can be reused.

@sergeuz sergeuz self-assigned this Jun 15, 2016
@sergeuz
Copy link
Member

sergeuz commented Jun 15, 2016

I've made several tests with both v1 and v2 library layouts and the patch worked as expected. Few comments:

  • Currently, you need to specify APPLIBSV1 and/or APPLIBSV2 variables rather than just APPLIBS, so the documentation is not correct in this aspect
  • Multiple libraries can be specified using whitespace between the library paths, e.g.: make -s APPDIR=path/to/app APPLIBSV1="path/to/lib1 path/to/lib2"
  • Does the v2 match some existing library layout, which we want to support for compatibility purposes? Typically, C/C++ libraries have public header files separated from source files using some directory like inc or include, while v2 requires everything to be in src directory.

@m-mcgowan
Copy link
Contributor Author

For v2, we are following the arduino libraries format. You're right - the docs haven't been updated after the conversation above clarifying the need for V2 support. It's mainly intended for machine use - particle compile --local will set up the correct env vars.

@m-mcgowan m-mcgowan merged commit 14b0173 into develop Jun 15, 2016
@technobly
Copy link
Member

👏

@technobly technobly mentioned this pull request Aug 25, 2016
7 tasks
@m-mcgowan m-mcgowan deleted the feature/vendorlibraries branch September 27, 2016 16:43
This pull request was closed.
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.

6 participants