Skip to content
This repository was archived by the owner on Mar 17, 2025. It is now read-only.

third-party: bump arduino-json to 5.2 #97

Merged
merged 3 commits into from
Apr 26, 2016

Conversation

proppy
Copy link
Contributor

@proppy proppy commented Apr 26, 2016

No description provided.

@@ -9,11 +9,8 @@

#ifndef ARDUINO

#ifndef ARDUINO_STRING_OVERRIDE
Copy link
Collaborator

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.

Copy link
Contributor Author

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? :)

Copy link
Collaborator

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.

See https://github.com/ed7coyne/arduino-mock/blob/0f7f9fa2cea822edf7b11099110b3e0fdb895a53/include/arduino-mock/Arduino.h#L103

Copy link
Collaborator

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.

Copy link
Contributor Author

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/

@proppy
Copy link
Contributor Author

proppy commented Apr 26, 2016

PTAL

@ed7coyne ed7coyne merged commit c4608b0 into FirebaseExtended:master Apr 26, 2016
@ed7coyne
Copy link
Collaborator

On a side note, why didn't travis run for this?

@proppy
Copy link
Contributor Author

proppy commented Apr 26, 2016

it did (click on the little green mark next to each commit): https://travis-ci.org/googlesamples/firebase-arduino/builds/125974556

@ed7coyne
Copy link
Collaborator

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants