Value returned by supportedPIDS_xx_xx() methods is off by one bit.

[jimwhitelaw]

Describe the bug Value returned by supportedPIDS_xx_xx() methods is off by one bit compared to the value that is actually returned by the ELM327 device.

To Reproduce Snippet of sample code used:

#include "BluetoothSerial.h"
#include "ELMduino.h"

BluetoothSerial SerialBT;
#define ELM_PORT SerialBT
#define DEBUG_PORT Serial

ELM327 myELM327;

uint32_t rpm = 0;

void setup()
{
#if LED_BUILTIN
    pinMode(LED_BUILTIN, OUTPUT);
    digitalWrite(LED_BUILTIN, LOW);
#endif

    DEBUG_PORT.begin(115200);
    // SerialBT.setPin("1234");
    ELM_PORT.begin("ArduHUD", true);

    if (!ELM_PORT.connect("ELMULATOR"))
    {
        DEBUG_PORT.println("Couldn't connect to OBD scanner - Phase 1");
        while (1)
            ;
    }

    if (!myELM327.begin(ELM_PORT, true, 2000))
    {
        Serial.println("Couldn't connect to OBD scanner - Phase 2");
        while (1)
            ;
    }

    Serial.println("Connected to ELM327");
}
void loop()
{
    uint32_t pids = myELM327.supportedPIDs_1_20();

    if (myELM327.nb_rx_state == ELM_SUCCESS)
    {
        Serial.print("Raw response (uint64_t): "); Serial.println(myELM327.response);
        Serial.print("Conditioned response (float): "); Serial.println(myELM327.conditionResponse(4, 1, 0));
        Serial.print("Returned value (uint32_t): "); Serial.println(pids);

        delay(10000);
    }
    else if (myELM327.nb_rx_state != ELM_GETTING_MSG)
    {
        myELM327.printError();
    }       
}

Expected behavior The received response, conditioned response, and value returned from the method should all be the same. They are not. Debug output:

Service: 1
PID: 0
Normal length query detected
Query string: 01001
Clearing input serial buffer
Sending the following command/query: 01001
        Received char: 4
        Received char: 1
        Received char: _
        Received char: 0
        Received char: 0
        Received char: _
        Received char: 9
        Received char: 8
        Received char: _
        Received char: 3
        Received char: A
        Received char: _
        Received char: 8
        Received char: 0
        Received char: _
        Received char: 0
        Received char: 1
        Received char: \r
        Received char: \n
        Received char: >
Delimiter found.
All chars received: 4100983A8001
Expected response header: 4100
Single response detected
64-bit response: 
        responseByte_0: 1
        responseByte_1: 128
        responseByte_2: 58
        responseByte_3: 152
        responseByte_4: 0
        responseByte_5: 0
        responseByte_6: 0
        responseByte_7: 0
Raw response (uint64_t): 2553970689
Conditioned response (float): 2553970688.00
Returned value (uint32_t): 2553970688

Potential cause I believe the issue arises in conditionResponse() either where the _uint64t response is multiplied by float scaleFactor or when the conditioned value (float) is returned. I think that one of those implicit casts to float is causing the LSB of the response to be dropped. I'm not sure how to fix at the moment, perhaps someone else knows the answer.

[PowerBroker2]

Could possibly fix by doing the following:

uint32_t ELM327::supportedPIDs_1_20()
{
    processPID(SERVICE_01, SUPPORTED_PIDS_1_20, 1, 4);

    return (((uint32_t)responseByte_3) << 24) |
           (((uint32_t)responseByte_2) << 16) |
           (((uint32_t)responseByte_1) << 8)  |
            ((uint32_t)responseByte_0);
}

What do you think?

[PowerBroker2]

Alternatively, it could be simpler to fix by doing this:

uint32_t ELM327::supportedPIDs_1_20()
{
    processPID(SERVICE_01, SUPPORTED_PIDS_1_20, 1, 4);

    return (uint32_t)response;
}

[jimwhitelaw]

Yes, I think the second method is the easiest way to fix these methods (isPidSupported() just uses "response" ignoring the conditioned output). However, it feels like a band-aid and I wonder if other methods that undergo a similar cast might also be affected? I'll need to do some more testing to see. Now that I think about it, I bet that the same thing happens with monitorStatus() but it goes unnoticed because we are only interested in the most significant byte and ignore the rest.

[PowerBroker2]

Fixed in 3.2.1