Make magnetometer calibration independent of locale (Fixes #452)

[thp]

Force the LC_NUMERIC locale to be the C locale, so that %f writes/reads . always for floating point (and not ,). As part of this change, also cleaned up the actual read/write code, as it was much more complicated than necessary.

(Historical tidbit: The original code was written by me nearly 10 years ago, see bde8a6a - quite silly in retrospect to do it like that when printf() and scanf() are perfectly capable of writing/reading verbatim characters...)

Not yet handled: Unsure how Win32 handles locales, it definitely doesn't seem to have uselocale() / newlocale() / freelocale() from a quick glance at mingw-w64 headers.

[thp]

For Win32, something like this could work, but I wasn't able to test it, as Wine doesn't implement _printf_l in its MSVCRT:

#include <stdio.h>
#include <locale.h>

int main()
{
    setlocale(LC_NUMERIC, "de_AT");

    printf("Hello, world! %f\n", 12.34f);

    _locale_t loc = _create_locale(LC_NUMERIC, "C");
    _printf_l("Hello, world! %f\n", loc, 12.34f);
    _free_locale(loc);

    return 0;
}

(build with i686-w64-mingw32-gcc -o test test.c)

This fails with:

% wine test.exe
Hello, world! 12,340000
wine: Call from 7BC2CEA0 to unimplemented function msvcrt.dll._printf_l, aborting

[nitsch]

For Win32, something like this could work, but I wasn't able to test it, as Wine doesn't implement _printf_l in its MSVCRT:

Yeah, it does. It has to be de-AT instead of de_AT to set the "wrong" locale, but the relevant part seems to work on Windows.

[thp]

Yeah, it does. It has to be de-AT instead of de_AT to set the "wrong" locale, but the relevant part seems to work on Windows.

Added the code. Let's see if it builds in CI.

[thp]

At this point, I'm quite tempted to just put the data into a struct, dump that struct to the calibration file and read the struct back from the calibration file as binary data (ignoring alignment and endianness issues, as it should be read back on the same machine, anyway, and between 32-bit and 64-bit x86 as well as 32-bit and 64-bit ARM, alignment and endianness are well-defined and should match up well enough for this to not be another pain point), as the OS-specific handling of this is ugly, and especially Win32 is the odd one out. Also, switching to a binary format without having backwards compatible code means people need to re-calibrate their controllers when upgrading the library.

I tried to avoid repetition by using macros, but it doesn't really improve readability.

If it builds like this, might merge it as-is.

[nitsch]

At this point, I'm quite tempted to just put the data into a struct, dump that struct to the calibration file and read the struct back from the calibration file as binary data (ignoring alignment and endianness issues, as it should be read back on the same machine, anyway, and between 32-bit and 64-bit x86 as well as 32-bit and 64-bit ARM, alignment and endianness are well-defined and should match up well enough for this to not be another pain point)

Yeah, I also considered this option when it became apparent that the whole special-casing was necessary to do it the right way [TM]. Storing the calibration data in human readable format is not actually of much use since nobody is going to tweak these values in a meaningful way anyway, I suppose. I would have tried to take care of the endianness, though (either by defining a fixed byte order for the file and parsing accordingly or by dumping a known word along with the struct as indicator). But you have a point there.

[thp]

At this point, I'm quite tempted to just put the data into a struct, dump that struct to the calibration file and read the struct back from the calibration file as binary data (ignoring alignment and endianness issues, as it should be read back on the same machine, anyway, and between 32-bit and 64-bit x86 as well as 32-bit and 64-bit ARM, alignment and endianness are well-defined and should match up well enough for this to not be another pain point)

Yeah, I also considered this option when it became apparent that the whole special-casing was necessary to do it the right way [TM]. Storing the calibration data in human readable format is not actually of much use since nobody is going to tweak these values in a meaningful way anyway, I suppose. I would have tried to take care of the endianness, though (either by defining a fixed byte order for the file and parsing accordingly or by dumping a known word along with the struct as indicator). But you have a point there.

Haha, we nearly got there, but in the end the mingw32 build failed, and while it'd probably be fine to drop 32-bit Windows support at this point, let's just go for the binary format instead. I've added a magic number at the start of the file, so in the future (not yet implemented) if we ever wanted to support reading calibration files from different endianness, we could add support there (the magic number's byte order would tell us whether or not we would need to byte-swap on read).

[thp]

Squashed some commits and fixed the comment. The locale-related commits I'll leave in the history if we never need to dig it out. Let's see if it builds, and would be good of somebody could test-drive the new code (don't have a Move setup here at the moment).

[thp]

CI seems to be happy now. @nitsch if you have time for a quick review/approval, please do (no rush). I'll merge after you're happy with the changes.

[thp]

Merged this after Alexander has closed this as completed: #452 (comment)