Obd2Ecu Fix transition to ON and retry if it fails#1350
Obd2Ecu Fix transition to ON and retry if it fails#1350frogonwheels wants to merge 2 commits intoopenvehicles:masterfrom
Conversation
- also puts loading the map into the task
|
Codewise, calling The changes to It's a bit concerning this doesn't address the assumed underlying issue in
|
|
I have seen the problem manifesting periodically where the HUD turns on but
the ecu just doesn't kick into gear so doesn't display the speed. Then
last weekend I went hunting in the logs and I did see an error with
mcp2515, though unfortunately I don't have it any more. I'll keep looking.
I was just assuming that it was something to do with it being 43 deg C
outside so probably 50+ inside the car for a couple of hours, and the BUS
was discombobulated from the experience.
I was unsure about continually retrying, maybe it needs to stop after a
bit?
I've realised we probably want some mode that indicates we've started the
task so it knows to kill it when we go to Stop if it didn't get to start
the bus properly
I'll add in that default case you mentioned, it makes sense.
//.ichael
…On Sat, 14 Mar 2026 at 18:52, Michael Balzer ***@***.***> wrote:
*dexterbg* left a comment
(openvehicles/Open-Vehicle-Monitoring-System-3#1350)
<#1350 (comment)>
Codewise, calling pcp::SetPowerMode() needs to be added to the default
case in mcp2515::SetPowerMode() to keep support for the other power modes
(even if not yet implemented in mcp2515).
The changes to obd2ecu look OK to me, but I have no device to test these
with.
It's a bit concerning this doesn't address the assumed underlying issue in
mcp2515 but makes coping with the issue / applying the workaround the
application's job. Start() generally may fail, but if retrying is
sometimes necessary just due to a hardware or driver bug, maybe it's rather
the driver that should take care of retrying.
mcp2515::Start() tries to detect error conditions, was this the case, did
you see some logs of that kind?
—
Reply to this email directly, view it on GitHub
<#1350 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABNANUMRECRPKXM4WPGGYD4QU2WDAVCNFSM6AAAAACWPUADP2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DANRQGI3TEOJUHE>
.
You are receiving this because you authored the thread.Message ID:
<openvehicles/Open-Vehicle-Monitoring-System-3/pull/1350/c4060272949@
github.com>
|
|
I have sorted a few things out, but I wonder if it is ok
that "obd2ecu.start" is only fired if the can is turned on, but before the
maps are loaded and the loop starts? "obd2ecu.stop" would only fire if
the start got fired.
//.
On Sat, 14 Mar 2026 at 19:50, Michael Geddes ***@***.***>
wrote:
… I have seen the problem manifesting periodically where the HUD turns on
but the ecu just doesn't kick into gear so doesn't display the speed. Then
last weekend I went hunting in the logs and I did see an error with
mcp2515, though unfortunately I don't have it any more. I'll keep looking.
I was just assuming that it was something to do with it being 43 deg C
outside so probably 50+ inside the car for a couple of hours, and the BUS
was discombobulated from the experience.
I was unsure about continually retrying, maybe it needs to stop after a
bit?
I've realised we probably want some mode that indicates we've started the
task so it knows to kill it when we go to Stop if it didn't get to start
the bus properly
I'll add in that default case you mentioned, it makes sense.
//.ichael
On Sat, 14 Mar 2026 at 18:52, Michael Balzer ***@***.***>
wrote:
> *dexterbg* left a comment
> (openvehicles/Open-Vehicle-Monitoring-System-3#1350)
> <#1350 (comment)>
>
> Codewise, calling pcp::SetPowerMode() needs to be added to the default
> case in mcp2515::SetPowerMode() to keep support for the other power
> modes (even if not yet implemented in mcp2515).
>
> The changes to obd2ecu look OK to me, but I have no device to test these
> with.
>
> It's a bit concerning this doesn't address the assumed underlying issue
> in mcp2515 but makes coping with the issue / applying the workaround the
> application's job. Start() generally may fail, but if retrying is
> sometimes necessary just due to a hardware or driver bug, maybe it's rather
> the driver that should take care of retrying.
>
> mcp2515::Start() tries to detect error conditions, was this the case,
> did you see some logs of that kind?
>
> —
> Reply to this email directly, view it on GitHub
> <#1350 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AABNANUMRECRPKXM4WPGGYD4QU2WDAVCNFSM6AAAAACWPUADP2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DANRQGI3TEOJUHE>
> .
> You are receiving this because you authored the thread.Message ID:
> <openvehicles/Open-Vehicle-Monitoring-System-3/pull/1350/c4060272949@
> github.com>
>
|
|
Those are user events, i.e. meant for scripting, there is no default listener in the firmware. The user guide documents these as being related to the OBD2ECU process (task) state, if your new logic still fits that usage, it should be fine. If scripts would need to be changed, you should rather introduce new events. |
|
I've been looking at other pcp based devices. There is a mix between
SetPowerMode() being the action and setting the state. Certainly as it
stands, calling SetPowerMode(On) and then having Start(..) fail, means that
GetPowerMode() == On.
The question is whether SetPowerMode(On) always means GetPowerMode() == On
? Or as I'm doing, is it conditional on it actually succeeding? Should
esp32can reflect this?
Should we be using action methods like Start() and Sleep() , DeepSleep()
instead?
For example SetPowerMode(Sleep) on mcp2515 & ecp32can will cause
"power.can?.sleep" and the "power.can?.off" since it calls
pcp:SetPowerMode(Sleep) and then calls Stop() which will call
pcp:SetPowerMode(Off).
Also what is the mess that is iterating through the values of m_mappm to
find the key which is the name of the state?
Anyway - moving NotifyStartup() to be conditional on the bus actually
starting for the obd2ecu, in fact keeps the events order relative to the
bus state events intact from how it was originally, so I think I'll do that.
//.
…On Sun, 15 Mar 2026, 01:36 Michael Balzer, ***@***.***> wrote:
*dexterbg* left a comment
(openvehicles/Open-Vehicle-Monitoring-System-3#1350)
<#1350 (comment)>
Those are user events, i.e. meant for scripting, there is no default
listener in the firmware. The user guide
<https://docs.openvehicles.com/en/latest/userguide/events.html> documents
these as being related to the OBD2ECU process (task) state, if your new
logic still fits that usage, it should be fine. If scripts would need to be
changed, you should rather introduce new events.
—
Reply to this email directly, view it on GitHub
<#1350 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABNANVYVRT5CHY3BVUZ6SL4QWKCJAVCNFSM6AAAAACWPUADP2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DANRQHE2TCMJZGU>
.
You are receiving this because you authored the thread.Message ID:
<openvehicles/Open-Vehicle-Monitoring-System-3/pull/1350/c4060951195@
github.com>
|
|
The PCP framework's primary intention is to provide a standard API & user interface to controlling the basic operational states of components. It does not require any specific semantics of the "on" state, "on" simply means "can be switched off or possibly put into sleep / deep sleep". Wether powering on can fail, keeping the previous PCP state, is up to the component, as is the general mapping of the states. Ideally, power state "on" means anything has been started or initialized that's actually using resources of any kind. "Sleep" mode then means telling the component to reduce resource usage if possible, "deep sleep" to minimize resource usage as far as possible, "off" means free all resources. While powering on may not be a sufficient initial operation without parameters for certain components (like the CAN buses), the PCP system may still be used to provide the standard shutdown/sleep operation API. After sleeping, powering on should ideally restore the pre-sleep state.
Well… could be solved otherwise, but as the map is already there and carrying a 1:1 relation, why not use it? |
Some logic of changing the state was not quite matched up.
Make sure that the obd started event only happens AFTER and IF the ecu has actually turned on.
Retries every second to restart OBD CAN if it doesn't turn on. At the moment it does NOT give up.