rogerclarkmelbourne / Arduino_STM32

Arduino STM32. Hardware files to support STM32 boards, on Arduino IDE 1.8.x including LeafLabs Maple and other generic STM32F103 boards
Other
2.5k stars 1.26k forks source link

dtostrf showing decimal houses even setted to don't have any #835

Closed fcig closed 3 years ago

fcig commented 3 years ago

Hi I recently updated the boards package to the lastest version and noticed an issue with dtostrf. It shows a number after the decimal point even when I set it to 0, to not show it.

I tracked the problem and it started with version 2020.11.9 where my skecth is unable to compile with this error: C:\Users\root\AppData\Local\Arduino15\packages\stm32duino\hardware\STM32F1\2020.11.9\cores\maple\avr\dtostrf.c:34:3: error: unknown type name 'uint8_t' uint8_t negative = 0;

With version 2020.11.14 and later the sketch is able to compile but it shows the mentioned problem. And with version 2020.11.2 it works as expected.

For example, with version 2020.11.2 and before it behaves this way: dtostrf(Temperature,3,0,text); The output is "100" , which is fine and expected.

But with version 2020.11.14 and later it behaves this way: dtostrf(Temperature,3,0,text); The output is "100.0" , the same output it gives if I use dtostrf(Temperature,3,1,text);

So I'm unable to disable the decimal number. I'm using a blue pill and Arduino IDE 1.8.13

stevstrong commented 3 years ago

I am not sure about the expected behaviour when you do not need positons after comma. But anyway, in this case you better first convert it to integer and then use itoa().

fcig commented 3 years ago

Yes, I don't see this as a huge issue, there are alternatives to reach this. I talked about the expected behaviour because I use this code with a variety of boards, mainly the Arduino AVR and now with stm32, and with AVR family the behaviour is to remove the decimal place like was happening with version 2020.11.2 and prior.

I don't use itoa in a first moment because I also use this code with a variety of displays and some of them don't have enough space in the screen. In these cases I can just reduce the number of decimal places.

fpistm commented 3 years ago

In fact Arduino does not well described this special case (prec = 0). http://www.nongnu.org/avr-libc/user-manual/group__avr__stdlib.html#ga060c998e77fb5fc0d3168b3ce8771d42

In sam core it is simply defined like this: https://github.com/arduino/ArduinoCore-sam/blob/790ff2c852bf159787a9966bddee4d9f55352d15/cores/arduino/avr/dtostrf.c#L23-L28

and so does not covered this case (and many other).

Anyway it seems coherent to manage this case even if in this case it simply requires to print an integer... Serial.println((int)Temperature); could do the stuff anyway the format of 3 digits will not be present. Note that dtostrf also managed rounding.

Output:

  3
3
  4

Sketch

void setup() {
  char text[10];
  double Temperature = 3.123456;
  Serial.begin(9600);
  Serial.println(dtostrf(Temperature, 3, 0, text));
  Serial.println((int)Temperature);
  Temperature = 3.6789;
  Serial.println(dtostrf(Temperature, 3, 0, text));
}

void loop() {
}

I've made a PR for this: https://github.com/stm32duino/Arduino_Core_STM32/pull/1268

fcig commented 3 years ago

Thanks for that. The format of 3 (or other numbers of) digits is something I forget to mention. With graphical displays the informations are normally left aligned, but if we want it right aligned is a pain because we need to set coordinates for each range, being one to 0-9 range, another to 10-99 range, and so on. dtostrf solves that reserving the places needed allowing just one coordinate for all ranges.

Also, I have some displays that the library only accepts chars (as it works directly with ASCII). In this case, even being a integer number I always need to convert it to text and dtostrf helped me with this aside taking care of rouding float numbers as mentioned.

fpistm commented 3 years ago

Welcome. Note that when I said I've made a PR for this this is not for this core but for STM32 core like the code is the same. So a PR could be done here based on this one. 😉

stevstrong commented 3 years ago

Why don't you simply use sprintf in your code instead of dtostrf? That could be used universally for different targets.

fcig commented 3 years ago

@fpistm no problem, thanks again!

@stevstrong yes, this is an alternative but I have multiple projects that are delivered to customers. It doesn't worth the effort to change them considering the risk of creating problems in a code that's already tested and I know it's working fine.

I used dtostrf simply because it was easier than sprintf for what I needed. For now it's easier for me to keep STM boards version 2020.11.2 and take note of this point in future projects.

As I pointed before it's not big issue for me, but in MHO I thinked the behaviour wasn't the expected and that's the reason I came here. If you and others here told me that the behavior I reported was normal, I would see no problem and would go after alternatives.

stevstrong commented 3 years ago

I think we have here an undefined behaviour. As the name says, dtostrf stands for "double to string", meaning that it expects a double variable, which is usually a 64-bit floating data type. If you are not using a floating type variable, you cannot ecpect this function to work as for long/integer type variables. That being said I am not motivated to change any core function just because is not conforming your expectations, which is actually a misuse of a function for another scope.

You said that you have multiple projects which may use different variable tyes. Accordingly, I think you always have to adapt anyway the code for specific targets, you cannot use exactly the same code. Hence it would make sense to either use itoa for non-floating variables, or change to sprintf because it would actually make it more universal.

fcig commented 3 years ago

Most cases I have are indeed with float numbers, from where I can decide how many decimal places are suitable for the display I'm currently using, and in some cases I need 0 decimals. What I posted in the first message is just an example. I understand your point and I respect it. If you think I'm wrong and misusing, ok, no problem. As I said before, I know alternatives to reach what I need and I don't have any problem to go this way. I came here just to ask because I thinked it would be a bug but you both explained that's an undefined behaviour and I'm ok with that. I understand and accept this fact. =)

stevstrong commented 3 years ago

Closing this as won't fix because of undefined behavior.