Use on embedded hardware

[CrustyAuklet]

I have been looking for a test framework that will work for host side and on target testing without compromising modernity. I found snitch and it seems to be exactly what i am looking for!

Our systems are C++20 and bare metal / RTOS but the main restrictions are just:

• no default new, only memory pool allocators
• no std::iostream, std::locale makes code size explode. We have a fork of iostream that removes locale.
• no exceptions, at least on the smallest devices we are using so far

With the customizable nature of snitch it was pretty easy to get it running, just a few things I had to patch and some things I noticed. So this issue is an FYI and if you are interested I can clean up and upstream my changes to you.

thread_local triggers a dynamic allocation (commit)

using thread_local triggers a dynamic allocation in newlib-nano. I don't know much about this, it was caught by an assert I have in _sbrk. since snitch doesn't appear to support sharding or other mutli-threaded things right now I took the easy route here and added a configure option SNITCH_WITH_MULTITHREADING, and when it is off just don't use thread_local

Using a custom snprintf (commit)

newlib-nano doesn't support %llu and just prints out lu instead of the actual number. there are at least half a dozen ways to solve this and I tried a few but the least intrusive one I landed on was to have a configuration option SNITCH_DEFINE_APPEND_FAST that is ON by default but when disabled excluded the contents of snitch_append.cpp. The program then has to provide implementations of the various append_fast functions.

Some smaller things I noticed but haven't addressed yet since they aren't an issue in normal flow of running tests:

• I get a compile error if SNITCH_MAX_EXPR_LENGTH and SNITCH_MAX_MESSAGE_LENGTH aren't the same, no conversion between a short string with the two different lengths
• terminate with message doesn't respect the customized print callback. Makes sense but on a embedded system normal IO isn't implemented.

[cschreib]

Thanks a lot for sharing your experience! I don't program embedded devices myself, but I always had this in the back of my head as a potential use case for snitch. My main worry was that weird compiler requirements might make it impossible to use C++20. Cool to see that it isn't the case, and I'm excited to see that snitch is indeed viable in this parallel universe, even if after a few tweaks.

The thread-local issue is definitely something we should merge, and your commit seems almost on the mark: it is missing the Meson support, as well as documentation in the README. I would also put the #define THREAD_LOCAL into a header for reuse, as #define SNITCH_THREAD_LOCAL to avoid collisions. Happy to accept a PR if you are willing!

The printing stuff is annoying. Are you able to use <charconv>? Or <format>? There's an issue and branch already adding support for std::to_chars from <charconv>, but it is somewhat blocked by the poor STL support for floating point (only available in very recent STD libs, which would limit adoption). This could be added as a configurable option for the "fast" (runtime) append():

1. use std::snprintf (current status quo)
2. use std::to_chars (Use std::to_chars instead of std::snprintf for converting numbers to string #18; faster, better behavior, but poorer STL coverage)
3. use snitch append_constexpr() (explored in Use std::to_chars instead of std::snprintf for converting numbers to string #18 as an alternative; slower in Debug, but fully portable)

Option 3 is the simplest, since the code is already there. It is just not as optimised as the STL stuff, so we currently branch out to the "fast" algorithm at runtime when possible for maximum speed. Option 2 would be ideal, but I don't think it has broad enough compiler support; so perhaps a mix of option 2 and 3 would be optimal: use std::to_chars if available, else fall back to append_constexpr().

[CrustyAuklet]

Willing to clean up the commit and open an MR on the THREAD_LOCAL fix

As far as the printing, falling back on the constexpr functions you have isn't bad. Having unit tests running on hardware is already pretty amazing and the performance hit wouldn't mean much compared to the bottle-neck of flashing the target.

I agree though that a mix of 2 and 3 would be best, and as support for std::to_chars improves then there is less of a difference. Do you have any ideas on how to detect if the floating point std::to_chars is available? I did a quick check and it doesn't look like __cpp_lib_to_chars helps, unless there are compiler specific values to check there. so I think you would either need to do compiler version checking with macros (gross but portable). Or lean on CMake feature detection. Maybe a CMake option, but have the default set based on CMake feature detection?

As far as the terminate_with_message not respecting the custom output, I was thinking I could forward stdout to my console. newlib provides weak functions to overload for these things, but I usually overload them with asserts to fail fast if people use C output instead of our custom stuff. For a test binary it is fine to change that policy though.

[cschreib]

As far as the terminate_with_message not respecting the custom output, I was thinking I could forward stdout to my console. newlib provides weak functions to overload for these things, but I usually overload them with asserts to fail fast if people use C output instead of our custom stuff. For a test binary it is fine to change that policy though.

Sorry I had forgotten about that last one! A simple fix would be to make terminate_with() use snitch::cli::console_print() (a function ref, which you can rebind) instead of snitch::stdout_print(). Would that work for you?

I get a compile error if SNITCH_MAX_EXPR_LENGTH and SNITCH_MAX_MESSAGE_LENGTH aren't the same, no conversion between a short string with the two different lengths

Probably a bug, will take a look. Edit 1: can't seem to be able to reproduce this. Can you show which values you used to trigger the problem, please? Or alternatively, send us a copy of your CMakeCache.txt (if you're using CMake). Edit 2: ah; got it. See #163.

[CrustyAuklet]

Sorry I had forgotten about that last one! A simple fix would be to make terminate_with() use snitch::cli::console_print() (a function ref, which you can rebind) instead of snitch::stdout_print(). Would that work for you?

I actually ended up creating customization points for my SDK to redirect standard streams. But I have worked in systems before where that isn't an easy option for various reasons. Since that function reference exists it would be best to use it IMO.

Searching around I only see it used in the testing of snitch, not within snitch. Do you think the test registry should set it's print_callback from snitch::cli::console_print? Nevermind, I see the registry is constinit 😄

so something like

int main(int argc, char** argv) {
    snitch::tests.print_callback = &my_print;
    snitch::cli::console_print = &my_print;
    std::optional<snitch::cli::input> args = snitch::cli::parse_arguments(argc, argv);
    if (args) {
        snitch::tests.configure(*args);
        return snitch::tests.run_tests(*args) ? 0 : 1;
    }
    return 1;
}

[cschreib]

I get a compile error if SNITCH_MAX_EXPR_LENGTH and SNITCH_MAX_MESSAGE_LENGTH aren't the same, no conversion between a short string with the two different lengths

This is fixed in #163.

[cschreib]

I actually ended up creating customization points for my SDK to redirect standard streams. But I have worked in systems before where that isn't an easy option for various reasons. Since that function reference exists it would be best to use it IMO.

This is done in #165.

so something like...

Yep!

[CrustyAuklet]

I get a compile error if SNITCH_MAX_EXPR_LENGTH and SNITCH_MAX_MESSAGE_LENGTH aren't the same, no conversion between a short string with the two different lengths

This is fixed in #163.

Awesome that you found this! I was just thinking that I owed a repro on this bug.

The tiny nit that I found that I failed to mention I think:

snitch/src/snitch_reporter_teamcity.cpp

Line 118 in 81cce09

append_or_truncate(string, static_cast<std::size_t>(duration * 1e6));

I think should use a float literal to avoid an implicit promotion of duration to a double:

append_or_truncate(string, static_cast<std::size_t>(duration * 1e6f));

I found this because I recently replaced the GCC option -fsingle-precision-constant
with -Werror=double-promotion and -Wfloat-conversion. This is important on embedded because ARM cortex doesn't have double precision floating point hardware and so any use of double will drag in software double routines.

here is an article on the topic if anyone is interested in more depth.

[cschreib]

Ah that's a good point! I didn't know that some platforms would emulate doubles in software... It's such a small change, I'll piggy back on one of the two open PRs. Thanks.

[cschreib]

I think all the reported issues have not been fixed in the main branch, and released as v1.2.5. Thank you for all the useful feedback!