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

Arduino Compatibility for All #1278

Closed
technobly opened this issue Mar 16, 2017 · 1 comment

Comments

@technobly
Copy link
Member

commented Mar 16, 2017

Re: https://community.particle.io/t/fastled-not-compiling-to-photon/30031/13?u=bdub

Legacy

To provide maximum compatibility for Arduino libraries, and still support hundreds of libraries that were ported from Arduino to Particle, we include Arduino defines by default for 0.6.1 firmware now.

In the past <= 0.6.0, users would get Arduino defines without the need to add #include "Arduino.h" to their code. And there was no Arduino.h, so that had to be commented out in libraries for them to compile.

Present 0.6.1

Now Arduino.h just includes Particle.h (which in turn includes the legacy application.h). Also legacy libraries would include application.h directly in many cases. If application.h is included, PARTICLE_ARDUINO_COMPATIBILITY will be defined. And by default it will be included and set to 1. If that is the case, all of the defines in spark_wiring_arduino.h will be included in your code. So RwReg will be defined.

There is no way to get ahead of the application.h include if it is included in a library and that is added to your app in the Web IDE. It appears to compile the library first.

If we could get the app to compile first, and compile the library once the include for the library is found, we could juggle something like this perhaps:

#include "Particle.h"

#ifdef RwReg
  #undef RwReg
#endif

#include "FastLED.h"
FASTLED_USING_NAMESPACE;
#define NUM_LEDS 60
CRGB leds[NUM_LEDS];
void setup() { FastLED.addLeds<NEOPIXEL, 6>(leds, NUM_LEDS); }
void loop() { 
    leds[0] = CRGB::White; FastLED.show(); delay(30); 
    leds[0] = CRGB::Black; FastLED.show(); delay(30);
}

But that's going to be a mess I think because there's bound to be a bunch of things to redefine.

Suggested Way Forward (for 0.6.1)

I think the proper thing to do here to maintain compatibility with the library and older AND newer firmware, is to wrap the arduino-based defines embedded in the library with

#ifndef MY_ARDUINO_SYMBOL 
  #define MY_ARDUINO_SYMBOL 
#endif

These defines would have been needed in the past for older firmware when we didn't have a lot of arduino defines by default. Now that we include everything and the kitchen sink, you don't need to include these any more.

If you have your arduino defines grouped in one place, a test for ARDUINO might be a good way to include them all. That way you don't have to put #ifndef on each of them, even though it's still a good idea.

So that might look like this:

#ifndef ARDUINO
  // add my arduino defines here to be included for firmware < 0.6.1-rc.2 !
#endif

Future Thoughts (for 0.6.2)

A way around some of the issues we are having now might have been to not include the new big list of arduino defines by default, but instead just include the small list of defines from 0.6.0 by default and require users to include #include Arduino.h to get the big list. That sounds easy enough, but still might be tricky depending on when Arduino.h is included. It would likely involve undef/redef of the small list of defines from 0.6.0. This could also allow above proposed changes to libraries for 0.6.1 to remain working for >=0.6.2 as the small list of defines did not include ARDUINO.


Completeness:

  • Minimum test case added
  • Device, system and user firmware versions stated

@technobly technobly added this to the 0.6.2 milestone Mar 16, 2017

@m-mcgowan m-mcgowan referenced this issue Mar 27, 2017
6 of 7 tasks complete
@technobly

This comment has been minimized.

Copy link
Member Author

commented Apr 7, 2017

Added to 0.6.2-rc.1 available in since 4/5/2017

@technobly technobly closed this Apr 7, 2017

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