Most CI fails are due to arduino-builder crapping out, but the PlatformIO tests do show problems that need fixing in the examples to make this patch work.

https://travis-ci.org/esp8266/Arduino/jobs/591524978

https://travis-ci.org/esp8266/Arduino/jobs/591524979

yep, I am still trying to learn how to replicate what travis is testing locally so that I can fix all of them in one go. for now this is an iterative approach :)

cd tests./run_CI_locally.sh

(edit: submodules are now automatically updated)

@devyte all API changes are in classes where their name starts with ESP8266. I would assume there is no 3rd party incompatibility here. There might be corner cases where ESP8266 code is affected, but I can't imagine this to be a very frequent case (to be honest I don't even know how you would notice String vs const String& other than reduced code footprint and better performance).

About CI, you can run mock trivial test locally with

cd tests/hostmake cleanmake  ../../libraries/ESP8266WebServer/examples/FSBrowser/FSBrowser

(fromtests/buildm.sh)

@devyte, the only changes I see in the externally visible*.h files are:

• type varname -> const type varname
• String x -> const String &x

Adding a const to the param seems absolutely safe, as the const flows down into the function but not upwards to the caller, right? A char* can be passed into a fcn requiring a const char * without any concern.

The & operator is a compiler structure hint, and other than passing by ref and not value user code should need no change, either. The compiler will simply emit apush addr instead ofmemcpy onto stack.

Granted, if you have a blob using these libs the 2nd change will cause trouble, but the ESPRESSIF ones do not use our libs, obviously and we've never guaranteed binary ABI compatibility.

The constness trouble was encountered by@d-a-v and was related to 3rd party libs that are meant to work across cores for diffrent uCs. In the end, he had to roll back.
Example: derive a class from one of these and override a method. In our core it's const, in some other core it's not const => 3rd party lib no longer compiles for both cores and requires extensive (additional) #ifdefing.

Like@dirkmueller said, these have the esp8266 prefix and I suspect the signature changes might be ok, but I don't see any assurance, which means this could be a breaking change.
I'm ok with moving forward, as long as the risk is understood: if a breakage report comes in this will need to be rolled back, and if there is a release in the middle, we'll have to make a new release with the rollback fix.

Ah, I see. The code as-is is safe, but there might be libs which might subclass them. IIRC,@d-a-v's issue was when he adjusted Arduino Client(?) and other libs which were subclasses of it crapped out.

On further thought, this is a breaking change any way you look at it, because any user with derived classes that override methods will need updating to add constness. I say target for v3.

@d-a-v overrides are only for virtual methods, which most of them are not. are you okay when we remove that change for public/protected virtual methods?

I think the only part that is a change in a virtual function signature is in RequestHandler.h which I think is actually unreleased. I'll double check this part.

My issues were mostly because of inheritance.
I don't think String is inherited by other projects.
That kind of changes is IMHO harmless when the class is used (vs inherited). And useful :)

sorry, wrong completion, I wanted to talk to@devyte

No harm, I was answering to@earlephilhower two posts earlier 😆
Anyway I don't see any "official" arduino classes in this PR

@dirkmueller I think so, but I don't want to just drop the rest either. Please split the virtual ones into a separate pr, and I'll target for v3.

@devyte splitted into#6583