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

Conversation

@m-mcgowan
Copy link
Contributor

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)
m-mcgowan added 2 commits May 25, 2016
external libraries support. This works by dynamically building new ma…
…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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2016

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

This comment has been minimized.

Copy link
Member

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.

m-mcgowan added 2 commits Jun 12, 2016
renamed APPLIBS to APPLIBSV1 and added support for v2 libraries via A…
…PPLIBSV2. V2 libraries build sources under the src/ folder under each library.
@m-mcgowan

This comment has been minimized.

Copy link
Contributor Author

commented Jun 12, 2016

testing (requires bats to be installed)

cd build/test
bats libs.bats
@mrmowgli

This comment has been minimized.

Copy link

commented Jun 13, 2016

+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 added 2 commits Jun 13, 2016
@m-mcgowan

This comment has been minimized.

Copy link
Contributor Author

commented Jun 15, 2016

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor Author

commented Jun 15, 2016

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

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@technobly

This comment has been minimized.

Copy link
Member

commented Jun 15, 2016

👏

@NGenetzky

This comment has been minimized.

Copy link

commented on docs/build.md in 646fd20 Jul 14, 2016

Missing ` to close code block.

@technobly technobly referenced this pull request Aug 25, 2016
7 of 7 tasks complete

@m-mcgowan m-mcgowan deleted the feature/vendorlibraries branch Sep 27, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.