Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for #203 batteryVoltage() returns zero #204

Merged
merged 30 commits into from
Dec 16, 2023

Conversation

jimwhitelaw
Copy link
Collaborator

Fix batteryVoltage = 0.

Payload is char array ending in V (ie, "12.5V") causing srttod to return zero. Fix removes the last char from payload so it converts properly.

Needs fix for parsing.
Next task, update foundCodes[] array properly.
Next do cleanup on debug messages
Copy link
Collaborator Author

@jimwhitelaw jimwhitelaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WTH am I doing wrong that every PR includes a whack of old commits!? This fix was only one commit and one new example program file.

@PowerBroker2
Copy link
Owner

Idk, maybe delete your branch and refork? I'll merge this for now and hopefully it won't break my last update.

@PowerBroker2 PowerBroker2 merged commit 5198be6 into PowerBroker2:master Dec 16, 2023
@jimwhitelaw jimwhitelaw deleted the fix-voltage branch December 16, 2023 22:27
@themelisx
Copy link

@PowerBroker2 In which version this will be available to platformio ?

@PowerBroker2
Copy link
Owner

I don't know how platformio or it's library manager works, so I'm not sure. If it mirrors the Arduino IDE's library manager, it will be available in the next tagged release. For now, you can use the main branch as it has these updates already.

@themelisx
Copy link

When you are coding in VS Code and you are using platformio, you just add the follow lines into the platformio.ini file and it just works. So I cannot use the main branch. anyway, thank you for your answer
lib_deps =
powerbroker2/ELMDuino

@jimwhitelaw
Copy link
Collaborator Author

I use platformio - you can work with any branch/version/commit of a library in a couple of ways:

  1. Specify the branch/commit/tag with #branch after the repo. Ie, lib_deps =
    https://github.com/powerbroker2/ELMDuino#master

  2. Clone the branch you want locally and then provide a full path to your local repo in your lib_deps. It doesn’t need to be a remote repo. lib_deps =
    /Users/jim/Documents/Platformio/Projects/ELMduino/src

First method is easiest if you just want to track a remote branch and use PIO tools to keep it up to date. Second method is easier if you want to be working on the lib itself.

@themelisx
Copy link

Good to know that.
Maybe it's a good idea to add this info to the readme file

@themelisx
Copy link

@jimwhitelaw I have tested your fix, indeed the "V" removed from your payload but the function still returns zero

@jimwhitelaw
Copy link
Collaborator Author

jimwhitelaw commented Jan 13, 2024

Can you please provide a sample of the code you are using and the debug (serial) output that is produced?

@themelisx
Copy link

@jimwhitelaw I am using the same code as described on the description when I open the ticket.
Below is a part of my first comment.
Ok, now the payload does not contains the "V" character but the library still returns always zero,

int newValue = int(obd->batteryVoltage() * 10);
so I am wating to get 131

Query ODB value
Clearing input serial buffer
Sending the following command/query: AT RV
Received char: A
Received char: T
Received char: _
Received char: R
Received char: V
Received char: \r
Received char: 1
Received char: 3
Received char: .
Received char: 1
Received char: V
Received char: \r
Received char: \r
Received char: >
Delimiter found.
All chars received: ATRV13.1V

@themelisx
Copy link

@jimwhitelaw I added some lines (take a look to the end of this post).
Seems that the strtod fails even if the "V" has removed

Sending the following command/query: AT RV
Received char: A
Received char: T
Received char: _
Received char: R
Received char: V
Received char: \r
Received char: 1
Received char: 1
Received char: .
Received char: 8
Received char: V
Received char: \r
Received char: \r
Received char: >
Delimiter found.
All chars received: ATRV11.8V
Voltage:
0.00

        nb_query_state = SEND_COMMAND;       // Reset the query state machine for next command
        payload[strlen(payload) - 1] = '\0'; // remove the last char ("V") from the payload value
        float test = (float)strtod(payload, NULL);
        Serial.println("Voltage:");
        Serial.println(test);
        return (float)strtod(payload, NULL);

@jimwhitelaw
Copy link
Collaborator Author

OK, I have found the issue and implemented a fix. I will do some additional testing with a couple ELM327 devices before I submit a PR with the change. Stay tuned...

@jimwhitelaw
Copy link
Collaborator Author

After some investigation, I found that some ELM327 devices respond to the voltage command differently than others. I have submitted a PR (#217) that handles both scenarios.

Also, I should note that the code you have provided may not do what you expect, even when batteryVoltage() is working properly. When the voltage value (a float) is multiplied by int value 10, the result is a float, but it only includes the integer portion of the original value. So the following code, given a voltage value of 13.1V:

int newValue = int(obd->batteryVoltage() * 10);

Will return 130, not 131. You could probably mitigate this with the following:

int newValue = (int)(obd->batteryVoltage() * 10.0)

DerKleinePunk pushed a commit to DerKleinePunk/ELMduino that referenced this pull request Jan 26, 2024
)

* Added currentDTCCodes() method - works for a single code

* Preparing for mulitple code responses

* First pass at multiple response support

* Implemented non-blocking version.

* Simplified code, implemented loop

* Modifications to support both blocking and non-blocking

* Fixed up check for number of codes present.

* Added bluetooth PIN

* Updates for actual returned DTC code format

* Working to fetch codes.
Needs fix for parsing.

* Fixes for parsing codes correctly.
Next task, update foundCodes[] array properly.

* Working with foundCodes arrray now.
Next do cleanup on debug messages

* Removed no longer needed debug statements

* Working as designed.

* Moved dtcResponse into ELMduino class

* Updated to get correct byte of monitorStatus() response

* Added MIL Status reporting

* Removed need to pass numCodes to currentDTCCodes()

* Added blocking call to currentDTCCodes()

* Updated comments

* Added method to check / retrieve current DTCs

* Added example program to check MIL (Check Engine Light) status and number of current DTC codes.

* fix for batteryVoltage() returns zero PowerBroker2#203
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants