sparkfun / Arduino_Apollo3

Arduino core to support the Apollo3 microcontroller from Ambiq Micro
83 stars 37 forks source link

printf does not handle floating point #278

Open paulvha opened 4 years ago

paulvha commented 4 years ago

Reproduce:

float value = 21.5; char symbol = 'C'; Serial.printf(" Temp: %2.2f%c ", ret, symbol);

will display : Temp: %2.2f Not Temp: 21.5C. Both the value and the symbol are not printed

Root cause In the different board variants, the file mbed_config.h, MBED_CONF_PLATFORM_MINIMAL_PRINTF_ENABLE_FLOATING_POINT is defined as zero. It is then excluded in mbed_printf_implementation.c.

regards, Paul

oclyke commented 4 years ago

Good catch - thanks!

oclyke commented 4 years ago

@paulvha from your other issue (rebuilding libmbed-os.a) I presume you have set about trying to enable floating point in the minimal printf lib. Have you had any luck with this? My own experience has been frustrating:

obviously it is not sufficient to simply change the mbed_config.h file in the Arduino core - the configuration must be present when mbed is compiled for the corresponding source code to be used

paulvha commented 4 years ago

I have spent the past week to rework an earlier Bluetooth implementation of AMDTP which I had build on the previous Library and to make it work now on ArduinoBLE and library 2.01. It took me a bit more time to get this going, but it is working now with an ATP running as a server and an EDGE or Ubuntu or Raspberry PI (with Bluez) running as a client. It is all posted on github. Finally tIme to look at this now.

paulvha commented 4 years ago

Can compile now. Need to load a number of module (and again and again and...)

I have used the mbed-os directory as with 2.0.1 as well as tried the github location that Wenn0101 pointed out. The resulted static library is not the same size as shipped. I have used the 9.0 GCC-ARM compiler as well as the 7.3.0 version, but I end up with a different size (both the same) than the static that is shipped with the 2.0.1 version. For now I assume an extra variable is applied somewhere. The mbed_config.h files are exactly the same as the one with 2.0.1 all the time.

Looking at mbed_printf_implementation.c, if TARGET_LIKE_MBED is NOT defined it will enable floating point, BUT if it is defined it will use a default configuration and then exclude floating-point. Not sure why..

The TARGET_LIKE_MBED looks to be defined as part of the symbols (ending up in .profile.cxx and .profile.c in BUILD ) in mbed_toolchain.py (around line 240).

I think the fast / best way forward is to change the default in the top of the mbed_printf_implementation.c to include floating-point.

regards Paul

paulvha commented 4 years ago

I was able to compile a static with MBED_CONF_PLATFORM_MINIMAL_PRINTF_ENABLE_FLOATING_POINT. I had to change in mbed-os/platform/mbed_lib_jason line 151 from false to true. It compiles without error and shows mbed_config.h set as 1. However, when trying to use this in 2.0.1 ( I compiled for the ATP) with a simple sketch ( Serial.begin(115200) and Serial.println(test)) it failes tocompile, as it was missing .cxx_flags and symbols. They are not in the BUILD location. I have copied these different files from the original mbed-folder for the ATP. It now compiles without error, but does not do anything. Retry with the mbed folder in ATP that is part of the 2.0.1, the sketch works. Where to get the other files from ??

paulvha commented 4 years ago

It is working now (I had to set the right variable path for the compiler). I have build the static with the as-is configuration. Run a simple sketch : Setup() { Serial.begin(115200); Serial.printf("hallo\n"); float a = 16.66; char symb = 'C'; Serial.printf("test float %2.2f%c\n",a,symb); }

As expected the output is : test float %2.2f%c , no float no symbol

Then applied the change in mbed-os/platform/mbed_lib_jason around line 151 from false to true and rebuild the static

The output then is : test float 16.660000C

As the mbed_print_implementation-code is written, the format (%2.2) is neglected and there will always be 6 digits behind the decimal point as this is detemined by BED_CONF_PLATFORM_MINIMAL_PRINTF_SET_FLOATING_POINT_MAX_DECIMALS.

oclyke commented 4 years ago

Thanks @paulvha, it is good to know of a possible workaround. I am still holding out for a solution that could be permanently incorporated into the mbed repo. https://forums.mbed.com/t/how-to-use-overrides-for-static-library-build/10489/2

If we don't get any info soon we can consider building your workaround into the CI builds.

P.s. sorry it comes too late to help you but those files are generated from the generate-variants workflow: https://github.com/sparkfun/Arduino_Apollo3/blob/080f528c75f478352b1f43abca9a218dd78a8be8/.github/workflows/generate-variants.yml#L87-L99

paulvha commented 4 years ago

Thanks! I see what you mean. The app-config parameter is NOT passed on to build.py (the program used for building the library) and thus the target_override will not work. I expect for now it will need the possible workaround and manual edit

I have tried in target.jason to add to FAMILY_Apollo3 : "printf_lib": ["std"] and created the library. Then you need to change the link-options and remove the different --wrap,printf etc. The sketch did compile and upload. But Serial.printf("test %2.2f gg", 16.6) only prints " test gg". That is because it is using the vsnprintf which is part of the GCC and is very limited. It would not be difficult to add an external "vsnprintf() ( e.g. https://www.arduinolibraries.info/libraries/lib-printf) and make the reference to that in hardwareserial.cpp/printf

Another thought is to accept this is a limitation for now, assuming the mbed will solve in the future. Then advise users to use for float multiple Serial.print() statements instead of a single Serial.printf (that is what I have done now in my sketches). It will also enable having the right number of digits behind the decimal point.

arinabykadorova commented 2 years ago

What is the status of this? Could someone please make printing floats as the default for the forked Sparkfun library? It is really hard to change, and seems like something the Artemis should just do.

paulvha commented 2 years ago

Based on the idea and link ( https://www.arduinolibraries.info/libraries/lib-printf) ) that I had shared in a previous post, I had created a printf that does handle the floating point (and other display) as defined Manual pages for Linux.

It requires the following 3 changes :

  1. Unzip the attached file and copy the folder 'printf' to library apollo3/2.2.1/cores/arduino/mbed-bridge/core-implement printf.zip

  2. Change in library apollo3/2.2.1/cores/arduino/mbed-bridge/core-implement/HardwareSerial.cpp (2 changes)

2A. In the top add (near line 7) : #include "printf/printf.h"

2B. The content of current 'printf' routine needs to change to


int UART::printf(const char *format, ...){
  va_list va;
  va_start(va, format);
  const int space = Artemis_printf(0, (size_t)-1, format, va) + 1;
  char buf[space];
  memset(buf, 0x00, space);
  int size = Artemis_printf(buf, space, format, va);
  va_end(va);

  write(buf, size);
  return size;
}

Now recompile your sketch and you have a complete working printf.

regards, Paul

arinabykadorova commented 2 years ago

Thank you! This works great for serial communications. Is there something analogous that I can do for I2C?

paulvha commented 2 years ago

Thanks . I am not getting the point about the I2C. Can you explain a bit more ?

regards, Paul

tkw722 commented 1 year ago

Hi All,

Jumping on the bandwagon here. I see that there are a couple workarounds in place, but what is the status of resolving this so no workaround is compiled? It would seem like double or float to string conversion should be something that works out of the box. Surprised this has been open for almost 3 years now.

Thanks, Tim

gigapod commented 1 year ago

Hi -

Our lead dev on Artemis/Arduino left last year - so issue repair has been lagging.

Dug into this issue this afternoon. This fix - changing the config file as noted above by @paulvha was easy enough, but the build and deploy takes work. This isn't my day job, but I believe I figured it out :).

I posted a "pre-release" with a fix for this here

To test -

Note: Formatting is always 6 decimal places - it's how mbed-os simple printf works.

Give this a test and see if it works for you. We'll test also, and if everything works, I'll get this published so it's available from Arduino IDE/CLI.

-Kirk