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

Reliability of stopPolling() with small ECUPoller intervals #4

Open
CJ-Davies opened this issue Jul 14, 2017 · 2 comments
Open

Reliability of stopPolling() with small ECUPoller intervals #4

CJ-Davies opened this issue Jul 14, 2017 · 2 comments

Comments

@CJ-Davies
Copy link

I am writing an application that uses obd-parser along with obd-parser-serial-connection to query an ELM 327 bluetooth adapter (currently connected to an Arduino with the Seeed Studio CAN-Bus shield serving as a hardware emulator).

I query the ECU & get the 8 PIDs that I have written support for in the Arduino sketch, then initialise pollers for these 8 PIDs. If I set the interval of each ECUPoller instance to something fairly slow like 500ms, I can start & stop them via startPolling() & stopPolling() with no problems. However if I set the interval to something faster like 200ms I can start them fine but when I try to stop them, some or all of them do not stop & instead continue polling. If I set the interval to something very fast like 50ms, then all of them fail to stop polling when I call stopPolling().

After adding debug console.log statements to obd-parser/lib/poller.js I can see that the stopPolling() function is in fact being called 8 times, but I cannot work out why some of these calls are not having the documented effect.

I put together a much simpler test file which simply starts one poller (Mode 01 PID 12/0x0C - RPM), increments a count each time a response is received, then calls stopPolling() once the count reaches 5. I added debug console.log statements to both the startPolling() & stopPolling() functions. In this test, the poller never stops polling?

Here is my test file --> https://gist.github.com/CJ-Davies/5a810072245a26bf99fb0a7893933a15

And the console output --> https://gist.github.com/CJ-Davies/4f334ec73f6159e96e08bac579d3cb5c

I hope I am just missing something obvious & this issue can be closed with 'problem exists between keyboard and chair'...

@CJ-Davies
Copy link
Author

I've just tested with a real car (2017 Golf R) & confirmed the same behaviour, so it doesn't seem to be an issue with my Arduino-based emulator.

@evanshortiss
Copy link
Owner

evanshortiss commented Oct 19, 2017

I hope I am just missing something obvious & this issue can be closed with 'problem exists between keyboard and chair'...

😂

@CJ-Davies I've not seen this issue, but I think I know what the cause could be. This line here is called when a matching poll response is returned from the ECU, but it doesn't check the value of this.polling first. So, let's say you have a timeline like so:

  1. Call startPolling and have an aggressive poll value such as 50ms
  2. First poll completes
  3. Next poll is queued
  4. Poll is written to ECU
  5. Call stopPolling
  6. ECU returns data and a new poll is triggered

The order of points 5 & 6 is the issue. The ECU returns the data after stopPolling is called, thus a new poll is triggered by the code I linked above since the callback does not check this.polling === false before calling doNextPoll(self.getNextPollDelay()). If the gap between polls is large enough you have a higher probability of calling stopPolling in the time window before a message is written to the ECU, but when it's narrow you are more likely to write it after the message has been written and a response is pending. Ideally I should use an FSM

Any interest in testing and submitting a PR?

PS - Nice car to own and/or work with!

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

No branches or pull requests

2 participants