They did this in the ESP8266 core for Arduino and it caused breakage due to Arduino's traditional macro-basedmin andmax supporting parameters of different types, butstd:min andstd:max not supporting this.

Examples:

• Can't compile using Core Version: 2.4.0 and Adafruit_PWMServoDriver.h esp8266/Arduino#4096
• USB Host Shield Library fails to compile using 2.4.0 esp8266/Arduino#4154
• adafruit mqtt esp8266 example error esp8266/Arduino#4382

Demonstration code that compiled before this change, but causes an error after:

void setup() {  byte foo = 42;  byte bar = max(foo, 100);}void loop() {}

sketch_apr10a:3:26: error: no matching function for call to 'max(byte&, int)'   byte bar = max(foo, 100);                          ^C:\Users\per\AppData\Local\Temp\arduino_modified_sketch_632562\sketch_apr10a.ino:3:26: note: candidates are:In file included from C:\Users\per\AppData\Local\Arduino15\packages\arduino\hardware\samd\1.6.21\cores\arduino/Arduino.h:80:0,                 from C:\Users\per\AppData\Local\Temp\arduino_build_865727\sketch\sketch_apr10a.ino.cpp:1:c:\users\per\appdata\local\arduino15\packages\arduino\tools\arm-none-eabi-gcc\4.8.3-2014q1\arm-none-eabi\include\c++\4.8.3\bits\stl_algobase.h:260:5: note: template<class _Tp, class _Compare> const _Tp& std::max(const _Tp&, const _Tp&, _Compare)     max(const _Tp& __a, const _Tp& __b, _Compare __comp)     ^c:\users\per\appdata\local\arduino15\packages\arduino\tools\arm-none-eabi-gcc\4.8.3-2014q1\arm-none-eabi\include\c++\4.8.3\bits\stl_algobase.h:260:5: note:   template argument deduction/substitution failed:C:\Users\per\AppData\Local\Temp\arduino_modified_sketch_632562\sketch_apr10a.ino:3:26: note:   deduced conflicting types for parameter 'const _Tp' ('unsigned char' and 'int')   byte bar = max(foo, 100);                          ^In file included from C:\Users\per\AppData\Local\Arduino15\packages\arduino\hardware\samd\1.6.21\cores\arduino/Arduino.h:80:0,                 from C:\Users\per\AppData\Local\Temp\arduino_build_865727\sketch\sketch_apr10a.ino.cpp:1:c:\users\per\appdata\local\arduino15\packages\arduino\tools\arm-none-eabi-gcc\4.8.3-2014q1\arm-none-eabi\include\c++\4.8.3\bits\stl_algobase.h:216:5: note: template<class _Tp> const _Tp& std::max(const _Tp&, const _Tp&)     max(const _Tp& __a, const _Tp& __b)     ^c:\users\per\appdata\local\arduino15\packages\arduino\tools\arm-none-eabi-gcc\4.8.3-2014q1\arm-none-eabi\include\c++\4.8.3\bits\stl_algobase.h:216:5: note:   template argument deduction/substitution failed:C:\Users\per\AppData\Local\Temp\arduino_modified_sketch_632562\sketch_apr10a.ino:3:26: note:   deduced conflicting types for parameter 'const _Tp' ('unsigned char' and 'int')   byte bar = max(foo, 100);                          ^

You can argue that the code that broke is bad code, but the impact of this change should at least be considered.

Good point. I've replaced theusing declarations with template functions supporting multiple types. These functions should behave the same as themin andmax macro, but they can also coexist with std::min and std::max.

This is a much needed PR for SAMD boards, I hope it will be merged soon. I tried the changes locally by pasting directly into my localArduino.h and it works very well. Without this change, basically any use of a library that uses std:: namespace is likely to fail due to collision betweenstd::min() andstd::max() and the #defines. I know you know this but from an end user perspective, I want you to know this is causing real pain that needs a fix.

@per1234

Any ideas when this is going to be merged? Currently, I am having to manually patch each time I use an Arduino Zero based board.

If I read the PR correctly, it now removes theabs andround macros when using C++ without providing alternatives?

Also, in theArduinoCore-API repo, this is also fixed in a similar way, but with even more reliable macros for C. Seearduino/ArduinoCore-API#85 for some discussion about this and other related changes.

@matthijskooijman Theabs andround macros are left unmodified (seehttps://github.com/arduino/ArduinoCore-samd/pull/399/files#diff-517f1981b375d8695337397cddc169d2R103-R104). I do agree that this problem has already been solved in other Arduino repositories, and has been for quite a while. I'd be happy to replace this PR with the changes in the ArduinoCore-API if that would improve the merge-ability?

The abs and round macros are left unmodified

They are, but they are now inside the#ifndef __cplusplus block, so not usable from C++ anymore.

I'd be happy to replace this PR with the changes in the ArduinoCore-API if that would improve the merge-ability?

I'm not sure, at some point all of this code should just be replaced by the API versions (seearduino/ArduinoCore-API#95), but I'm not sure what the progress on that is. Also, I'm unsure if any such changes would still be merged here separetely, that's something for the Arduino devs to answer, I guess.

As far as I can see your proposed changes have made it intoArduinoCore-API. Since#567 has been merged you should have your desired functionality. Please reopen if you've got a different opinion.

@aentinger IIUC#567 only adds automated testing in preparation of merging ArduinoCore-API here, right? I don't see the API used in master yet?

Regardless, I think this can be closed, as the PR as-is is not ideal and will probably not be merged with an upcoming -API merge.

My bad, I linked the wrong PR. In fact,#560 which was merged just yesterday, broughtArduinoCore-API toArduinoCore-samd. Care to give it a spin?

Ah, I see that the files from ArduinoCore-API have been imported verbatim now, rather than in anapi subdirectory was was previously suggested for another core (AVR maybe), that's why I didn't see it. Cool to see this is merged, I'll be sure to test this when I do some SAMD work again (lots of STM32 for me lately, though).

Ah, I see that the files from ArduinoCore-API have been imported verbatim now, rather than in an api subdirectory was was previously suggested for another core (AVR maybe), that's why I didn't see it.

Hi 👋 that's not correct. The API needs to be symlinked in into anapi subdirectory. If you'd check out the core from master now it wouldn't compile if you don't symlink in the api. In the next packaged core release naturally the api folder will be available within the release, as requiring users to manually symlink would be a bit of a violation of Arduino philosophy.

Right, I didn't look close enough again. I see how it works now, thanks!