Arduino-PID-Library icon indicating copy to clipboard operation
Arduino-PID-Library copied to clipboard

basic example uses analogWrite(outputpin,DOUBLE) !

Open StefanL38 opened this issue 1 year ago • 6 comments

Hi everybody,

the example https://github.com/br3ttb/Arduino-PID-Library/blob/master/examples/PID_Basic/PID_Basic.ino defines output as double (=double-precision floating-point)

//Define Variables we'll be connecting to
double Setpoint, Input, Output;

and then uses this floating-point-variable in analogwrite

analogWrite(PIN_OUTPUT, Output);

that is a BIG BUG !

StefanL38 avatar Jan 04 '23 19:01 StefanL38

I'm curious as to what problem this causing? Most of the time analogWrite is not better than 12 bits and a single precision float has 23 bits of mantissa.

JacobChrist avatar Jan 04 '23 20:01 JacobChrist

The representation of a float is totally different than the representation of an integer image

https://forum.arduino.cc/t/how-to-covert-4-bytes-to-float/612320 How can this representation ever work with a function that expects an integer??

StefanL38 avatar Jan 05 '23 18:01 StefanL38

When you pass a floating point variable as a function argument where the function expects an integer, the compiler will automatically promote the float to an integer as if you had assigned a float to an integer variable. You can find details here, section 6.5.2.2 paragraph 7: https://stackoverflow.com/questions/1255775/default-argument-promotions-in-c-function-calls

When you assign a float to an integer variable in C, the result is that the fractional part of the float is discarded (truncated), so the number is rounded down. If that truncated result is too large to be represented by the integer it's being assigned to, the result is undefined behavior, which generally means it depends on the compiler. https://learn.microsoft.com/en-us/cpp/c-language/conversions-from-floating-point-types?view=msvc-170

In this sketch, if we assume the values of the float don't get larger than the integer type can represent, the result of the type conversion is just that we discard the fractional part of the value when writing it to the pin.

ottobonn avatar Jan 05 '23 21:01 ottobonn

Another way to put what ottobonn is saying is that the compiler automatically type casts the float to an int when passed to the function. So the effective code is:

    analogWrite(PIN_OUTPUT, (int)Output);

JacobChrist avatar Jan 05 '23 22:01 JacobChrist

@StefanL38 My original question to you was "I'm curious as to what problem this causing?". Because a float or a double can be much larger or smaller than what an int can hold, so when it is converted to an int there could be an issue. But I suspect that you haven't tried to code because I know it worked for me the last time I tried it.

I do understand why your confused. But to get the bits to align the and they way your thinking you would actually need to do some funny casting, maybe something like this:

int i = *((int *)&Output);

The above code is not tested, but this would probably line up all the float bit such that they would not make sense as an integer. But this is good to think about since if you want to transmit a float over a serial line you can convert it to an integer, then back.

float f = *((float *)&i); 

JacobChrist avatar Jan 06 '23 00:01 JacobChrist

Two things to remember:

  1. The PID output, which will be sent to the write function, is restricted by the SetOutput Limits function which, not-coincidentally, defaults to 0.0<->255.0
  2. This code has been in service for 10-15 years, and has used in many (many) projects. Unless there has recently been a change in how the compiler works, i'm having a hard time seeing how a fundamental flaw could have laid dormant until NOW.

On Thu, Jan 5, 2023, 7:06 PM Jacob Christ @.***> wrote:

@StefanL38 https://github.com/StefanL38 My original question to you was "I'm curious as to what problem this causing?". Because a float or a double can be much larger or smaller than what an int can hold, so when it is converted to an int there could be an issue. But I suspect that you haven't tried to code because I know it worked for me the last time I tried it.

I do understand why your confused. But to get the bits to align the and they way your thinking you would actually need to do some funny casting, maybe something like this:

int i = *((int *)&Output);

The above code is not tested, but this would probably line up all the float bit such that they would not make sense as an integer. But this is good to think about since if you want to transmit a float over a serial line you can convert it to an integer, then back.

float f = *((float *)&i);

— Reply to this email directly, view it on GitHub https://github.com/br3ttb/Arduino-PID-Library/issues/129#issuecomment-1372953952, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACYX4ULMVJRNAG6ATXFWXDWQ5OZLANCNFSM6AAAAAATREO2RU . You are receiving this because you are subscribed to this thread.Message ID: @.***>

br3ttb avatar Jan 06 '23 00:01 br3ttb