Skip to content
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

Enable building for platforms other than Arduino. #69

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

codygray
Copy link

@codygray codygray commented Jan 7, 2020

With a few minor tweaks, this library can be made more universal, so that it can be built for platforms other than Arduino. This vastly increases the usefulness of the library, without compromising any functionality.

This merge request contains the changes needed to make the library universal enough to be built for any platform with a standards-compliant C++11 compiler. These changes are as minimally-invasive as possible, and remain fully backwards-compatible (as long as ARDUINO is #defined, the original code will be used).

@codygray codygray mentioned this pull request Jan 7, 2020
@feliwir
Copy link

feliwir commented Jan 7, 2020

Would it maybe be possible to provide a millis function through a std:::function? On my STM32 chrono doesn’t work

@codygray
Copy link
Author

Would it maybe be possible to provide a millis function through a std:::function? On my STM32 chrono doesn’t work

Sure, you could modify the header file to accept an implementation in the form of a caller-specified lambda. That probably makes more sense than attempting to detect the environment with preprocessor macros, since the way of doing this tends to vary widely across different microcontrollers and development environments.

But I would certainly prefer to use the standard implementation whenever possible for simplicity and elegance, so I'm not quite sure how to merge this in alongside what I have now. Perhaps checking for C++11 support with a macro? That would work, unless your compiler reports it is C++11-compliant, yet omits support for the std::chrono libraries. And without C++11 support, you're not going to have std::function or lambda support, either.

For compatibility with these older environments, it might make more sense to just link in your own millis() function. I don't know. Open to suggestions on this.

@mikalhart
Copy link
Owner

Plan to introduce a millis() function for non-Arduino platforms using std:chrono (where it is supported). Would be grateful if someone could do a PLATFORMIO test on the result.

@robertlipe
Copy link

A lot changes in 4 years. I'd be surprised to find anything that ISN'T C++11 by now, but some systems may not provide a full implementation of modern C++ libraries. There's a lot in chrono https://en.cppreference.com/w/cpp/header/chrono, but chrono itself has been around for almost 15 years at this point. https://en.cppreference.com/w/cpp/11

To simplify the "where am I" choice, this came in formally around C++14 or so, but GCC has supported the has_include since like GCC 8 or something pretty far back. It's been in clang, like, forever. So depending on how paranoid you want to be, you can write

#if defined(__has_include)
#if __has_include()
// write in C++
#else
#write in arduino-ese using things like byte instead of uint8_t which works everywhere interesting.
#endif
#endif

Of course, if your system doesn't have a working chrono implementation, it won't have that header.

You can similarly test for for mathematical constants and use std::numbers::pi that'll give it to you in a constexpr, suitable for inlining AND showing in a debugger sensibly, like #defines often don't. Why use the system's constant? Because it knows things like the appropriate underlying type. For example, it's usually better to use a float on ESP32 where you have hardware float, but software-emulated doubles or a desktop where a 64bit (or 80!) long double is better. Similarly, an inline funcion that multiplied it by 2 will be treated appropriately by the optimizer but allow setting breaktpoints in it, exactly like a #define doesn't.

This should let your library run more easily on desktop computers where you can more easily build unit tests and, if needed, mock the time functions.

I think your stubbing in of millis() is exactly the right approach. Use standard C++ everywhere you can and without naming them (this would work on all the RISC-V platforms I have or Nuttx, for example, which provides zero or more of uClibc and LLVM's uclibc++ )

It's my experience, for example, that it goes down better socially to just provide a 'byte' type for code with a strong Arduino heritage than it is to try to "fix" that code because 'byte' isnt' a type. Sprinkling system_clock::now() around instead of building up a milli() seems to be met with less resistance.

BTW, POSIX spells HALF_PI as M_PI_2 per https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/math.h.html and IME, this is pretty well entrenched on even the most embedded of embedded platforms.

By combining these tips, your code can run anywhere (that matters) without building up a tangle of platform-specific #ifdefs by name. If it were my project, I'd take this PR with only a few minor accent tweaks (M_PI_2, , not testing for headers by platform name), but I'm not a maintainer. I'm just in the peanut galley reviewing code tonight to try to help out some PRs that seem to be stuck.

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

Successfully merging this pull request may close these issues.

5 participants