-
Notifications
You must be signed in to change notification settings - Fork 13.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clean up use of "byte" as a type. uint8_t or (C++17) std::byte are better #8090
Conversation
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.
Not a fan of the way Arduino did it, but this breaks Arduino compatibility.
uint16_t makeWord(uint16_t w);
uint16_t makeWord(byte h, byte l);
Removing byte elsewhere seems fine, but the 2-param makeword seems like it should be moved from unsigned char
to byte
in WMath.cpp
.
In the light of even, say:
using
lets the compiler cast and then link just fine. My reckoning in this is to get rid of "byte" as much as possible - rationale (byte is used as object ident) was given above. By the way, I messed up, doing the ESP32 in kind of at the same time, and didn't fix the return type inconsistency here in ESP8266 Core. I have done that now (fixup and rebase). There are other indications that this discussion is moot, like (AVR Arduino.h to ESP8266's):
and the AVR |
1f3a19b
to
8b48530
Compare
Regarding Arduino compatibility: https://github.com/arduino/ArduinoCore-API |
a5b8112
to
c00faa0
Compare
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.
LGTM
Change byte to uint8_t to cope with breaking change in 3.2.0 of platform: esp8266/Arduino#8090
Related to #8089