-
Notifications
You must be signed in to change notification settings - Fork 492
Conversation
@@ -9,11 +9,8 @@ | |||
|
|||
#ifndef ARDUINO | |||
|
|||
#ifndef ARDUINO_STRING_OVERRIDE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change I made by hand to 5.1.1 we will need to make it again unless we can think of any other clever hacks.
Basically if you are not running on an Arduino board it will try to define the arduino String class for it's local tests. The issue is that we do the same thing for our tests and they will conflict.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't we just not do it in our test? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, since they are just typedefing I assume they are using strings in a very basic way (probably just c_str()). Where we actually call methods on arduino's String and I built a wrapper that adapts std::string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other option that I think would work (been a while since I have been directly building large projects) but I think if we built arduino-json as a library than it wouldn't matter if there was a naming conflict, their code would use their typedef and our code would use our override.
I need to do more research on that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
restored the patch, and also added a .patch
so we can easily apply it on every new version.
Best would be to submit a PR for it on https://github.com/bblanchon/ArduinoJson/
PTAL |
On a side note, why didn't travis run for this? |
it did (click on the little green mark next to each commit): https://travis-ci.org/googlesamples/firebase-arduino/builds/125974556 |
Oh, I am used to seeing the larger box but I guess that only shows up when there are issues or it is still running. |
No description provided.