PowerBroker2 / ELMduino

Arduino OBD-II Bluetooth Scanner Interface Library for Car Hacking Projects
MIT License
635 stars 123 forks source link

When does `queryPid` return `true`? #248

Open Ryccoo opened 5 months ago

Ryccoo commented 5 months ago

Looking at the code for queryPid method, the docs states:

https://github.com/PowerBroker2/ELMduino/blob/5d041617e79bad7487e69f722f8719f9f6ef4155/src/ELMduino.cpp#L575-L579

It seems to return the value of connected

https://github.com/PowerBroker2/ELMduino/blob/5d041617e79bad7487e69f722f8719f9f6ef4155/src/ELMduino.cpp#L579-L585

Looking into the sendCommand method that is called by queryPid

https://github.com/PowerBroker2/ELMduino/blob/5d041617e79bad7487e69f722f8719f9f6ef4155/src/ELMduino.cpp#L2138

it sets connected to false https://github.com/PowerBroker2/ELMduino/blob/5d041617e79bad7487e69f722f8719f9f6ef4155/src/ELMduino.cpp#L2146

never setting it back to true.

This looks like queryPid would always return false ?

PowerBroker2 commented 5 months ago

There are a couple places where the class can detect a good connection:

In get_response() we have: https://github.com/PowerBroker2/ELMduino/blob/5d041617e79bad7487e69f722f8719f9f6ef4155/src/ELMduino.cpp#L2313

And in initializeELM() we have: https://github.com/PowerBroker2/ELMduino/blob/5d041617e79bad7487e69f722f8719f9f6ef4155/src/ELMduino.cpp#L126 https://github.com/PowerBroker2/ELMduino/blob/5d041617e79bad7487e69f722f8719f9f6ef4155/src/ELMduino.cpp#L148 https://github.com/PowerBroker2/ELMduino/blob/5d041617e79bad7487e69f722f8719f9f6ef4155/src/ELMduino.cpp#L169

PowerBroker2 commented 5 months ago

Looking closer, it seems queryPid() returns before calling get_response(), so it will return false all the time. Let me look into this and see if there's a good work around. I may simply turn queryPid() into a void return function since the determination as to if the ELM327 is connected or not happens during the parsing - not querying. Therefore returning the connected status may be irrelevant here.

Ryccoo commented 5 months ago

Yeah that is the case, the docs threw me off * bool - Whether or not the query was submitted successfully and also I was using an example from 4 years ago that someone else made, which was prior to the non-blocking / blocking commands, when queryPid used to have get_response code still in it, meaning it read stuff and also returned proper boolean

jimwhitelaw commented 4 months ago

@Ryccoo I just committed a fix for this inconsistency. As suggested by @PowerBroker2, queryPID() now returns void. The code comments and documentation now reflect this. Thanks for finding the issue.