Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Move to Arduino API 10501 #2797

Merged
merged 5 commits into from
Feb 9, 2025
Merged

Move to Arduino API 10501 #2797

merged 5 commits into from
Feb 9, 2025

Conversation

earlephilhower
Copy link
Owner

Track upstream Arduino API headers

@earlephilhower earlephilhower merged commit c79e543 into master Feb 9, 2025
26 checks passed
@earlephilhower earlephilhower deleted the newapi branch February 9, 2025 06:45
Comment on lines +163 to +165
#ifdef __cplusplus
using namespace arduino;
#endif

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

Is it possible that this namespace introduction causes this build failure in many projects ?

Example: https://github.com/ayushsharma82/NetWizard/actions/runs/13421632127/job/37495411747#step:15:2

I had to fix the CI to 4.4.3 for ESPAsyncWebServer: ESP32Async/ESPAsyncWebServer@306dcf6

Thanks!

@earlephilhower @ayushsharma82

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose it's possible to cause a problem, but very unlikely because in the prior release there was a blanket using namespace arduino in the API headers under Print. arduino/ArduinoCore-API@db53d3d So it's always been there, just moved to our own file and not the API sources with this PR (I forked the API when the project was just started).

The removal in the latest Arduino APIs actually caused problems with libraries here (TinyUSB off the top of my head). I just re-instated it here so it's effectively a no-op.

There were some API additions/changes with String (and bug fixes, the reason for moving fwd) so I think they're the reason CI is failing for you there, not the using statement. GCC knows to use the arduino::String, but can't figure out a cast that will work with the code in that file, it seems. Extra explicit casting might help you there...

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually looks like you can just explicitly make etag a String and make progress (bad hand written DIFF follows...)
https://github.com/ESP32Async/ESPAsyncWebServer/blob/306dcf6740018eee8d34581aa0464992f62aaff8/src/WebHandlers.cpp#L225

- etag = request->_tempFile.size();
+ etag = String((int)request->_tempFile.size());

Copy link

@mathieucarbou mathieucarbou Feb 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually looks like you can just explicitly make etag a String and make progress (bad hand written DIFF follows...)

I know the workarounds, the issue is more the break in backward compatibility in a patch digit upgrade.
That's not normal.
I do not understand why the overloaded operator is not found like with 4.4.3.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants