[Ovmsdev] OVMS Poller module/singleton

Thu May 2 21:23:24 HKT 2024

Thanks.

On that race condition... do you think we are better to just delete the
task  or I saw one recommendation that
We can delete it outside the while loop in the task itself!

//.ichael

On Tue, 30 Apr 2024, 17:46 Michael Balzer via OvmsDev, <
ovmsdev at lists.openvehicles.com> wrote:

> OK, understood & merged, thanks for the quick fix/workaround.
>
> I'll ask the Twizy driver to test this.
>
> Regards,
> Michael
>
> PS: side note: there's a shutdown race condition arising from `while
> (!m_is_shutdown)` in the new `OvmsVehicle::VehicleTask()`: if the task
> receives a message while m_is_shutdown already has been set, it will exit
> the loop and return from the task function, which isn't allowed. I think
> FreeRTOS will abort in that case.
>
>
>
> Am 30.04.24 um 08:55 schrieb Michael Geddes:
>
> No it will start the poll thread of the poller fine for those that don't
> use it.  That's the last commit.... so that should be covered. When the
> callback is registered the thread is force started.
>
>
> This is not to say that what you are suggesting isn't a better way
> forward... just that it should work.
>
> Michael
>
>
>
> On Tue, 30 Apr 2024, 14:51 Michael Balzer via OvmsDev, <
> ovmsdev at lists.openvehicles.com> wrote:
>
>> Michael,
>>
>> (taking this back to the list, as other developers may have helpful ideas
>> or suggestions)
>>
>> from a first check, your new patch (PR #1008) won't help for vehicles
>> that don't use the poller, like the generic DBC vehicle or the Fiat 500
>> (plus possibly non-public third party vehicles). As the poller is normally
>> compiled in, these would need a run time switch to re-enable the vehicle
>> task, or more suitably default into that configuration and switch to the
>> poller task only when the poller gets initialized.
>>
>> How about moving the task back into the vehicle class, but keeping your
>> task message extensions while doing so, and adding a way for the poller to
>> hook into the task? The dedicated vehicle task is an essential part of the
>> vehicle framework, but I do remember some occasions where a general way to
>> pass custom messages to the vehicle task would have been helpful to use the
>> task for more than CAN processing.
>>
>> If the poller shall become avaible as a separate service that can be used
>> without any vehicle instance (currently not a defined use case), the
>> construct of a CAN processor task that can be extended for custom messages
>> (or a message processor task that can subscribe to CAN frames) could be
>> factored out into a dedicated class or template. The poller could then
>> create his own instance of that class, if it cannot hook into an existing
>> one.
>>
>> Btw, in case you're not aware of this, FreeRTOS also provides queue sets (
>> https://www.freertos.org/RTOS-queue-sets.html). We haven't used them in
>> the OVMS yet, but they could be useful, especially if message unions become
>> large or tasks shall be able to dynamically subscribe to different message
>> sources.
>>
>> Regards,
>> Michael
>>
>>
>> Am 30.04.24 um 01:20 schrieb Michael Geddes:
>>
>> It might be worth reverting, but, I've got a patch suggestion that I'll
>> push up which will let me know if I understand everything and which might
>> provide a solution.
>>
>> If this isn't going to work then revert. (P/R coming)
>>
>> While the poller was still a part of the vehicle class it was probably
>> still all ok.   The poller had taken over what I assume you are talking
>> about as the vehicle task.  A call-back fromthe poller was calling
>> IncomingPollRxFrame which was then coming from the (now) poller task (is
>> that correct?)
>>
>> While we have OVMS_COMP_POLLER config defined, we could just use the
>> poller task to provide the IncomingPollRxFrame call-back from the poller
>> (was vehicle?) task.
>>
>> The problem is when OVMS_COMP_POLLER is undefined, we need an alternate
>> (maybe we could use the 'Event' loop then) which is when I hacked that bad
>> solution.
>>
>>
>> //.ichael
>>
>>
>> I was thinking though that because everything is being queued we could
>> divert some calls into the car
>>
>> On Mon, 29 Apr 2024, 18:58 Michael Balzer, <dexter at expeedo.de> wrote:
>>
>>> Michael,
>>>
>>> I've found a severe design flaw with your poller change. Sorry, I should
>>> have seen that before, and shouldn't have merged this.
>>>
>>> You have moved the standard CAN frame processing from the vehicle task
>>> into the CAN task by changing the vehicle from a CAN listener to a CAN
>>> callback processor. That will break all kind of things, vehicles like the
>>> Twizy, Smart and many others rely on frame processing being done in the
>>> dedicated vehicle task.
>>>
>>> This especially induces issues regarding CAN processing capacity, as the
>>> vehicles rely on having a separate context and not needing to decouple
>>> complex processing of incoming frames. So this will degrade the CAN
>>> processing performance seriously in many cases, where additional steps
>>> involving file or network I/O need to be done.
>>>
>>> So this needs to be changed back.
>>>
>>> I assumed you introduced a new dedicated poller task but kept the
>>> vehicle task intact. From your naming (OvmsVehicle::IncomingPollRxFrame),
>>> it seems you misinterpreted the vehicle task's purpose as only being used
>>> for polling.
>>>
>>> I assume this isn't a small change…? If so we should revert the poller
>>> merge, to avoid having a defunctional edge build state until the fix.
>>>
>>>
>>> Secondly: logging is generally expensive regardless of the log level
>>> control and log channels enabled. Any log message needs to be queued to the
>>> log system, so involves a lock and a potential wait state (for the queue) &
>>> resulting context switch (= minimum delay of 10 ms). Therefore, logging
>>> must be avoided as far as possible in any time critical context, and is
>>> forbidden under some circumstances, e.g. in a timer callback (see FreeRTOS
>>> docs on this). The log system load multiplies with connected web clients or
>>> enabled file logging and enabled debug / verbose level logging -- that
>>> quickly becomes too much, usually triggering the task watchdog.
>>>
>>> Your logging of nearly all bus events passing to/from the poller
>>> (especially the log entry in Queue_PollerFrame) becomes an issue on any
>>> vehicle that has high frequency running process data frames on the bus,
>>> like the Twizy or Smart. As a counter measure, I've just added a runtime
>>> control for all the poller's verbose logging (by default now off), and
>>> changed some debug level logs to verbose. Not sure if I've catched all that
>>> may need to be silenced by default. Please check all your log calls and
>>> place them under the new "trace" control flag wherever appropriate.
>>>
>>> This won't help avoiding issues with process data frame buses though.
>>>
>>>
>>> Shall I revert the poller merge for now?
>>>
>>> Regards,
>>> Michael
>>>
>>>
>>>
>>> Am 29.04.24 um 06:18 schrieb Michael Geddes:
>>>
>>> Btw I included the log changes in the p/r which is a few small commits
>>> for the poller.
>>>
>>> //.
>>>
>>> On Mon, 29 Apr 2024, 07:20 Michael Geddes, <frog at bunyip.wheelycreek.net>
>>> wrote:
>>>
>>>> Hey Michael,
>>>> I've got to work now (it's Monday), but I suspect those 'giving up' are
>>>> from unsolicited messages on the bus.
>>>> I can re-order things so that the message will likely be  'dropped (no
>>>> poll entry)' rather than the time-out message.
>>>> And make it a verbose log.
>>>>
>>>> //.ichael
>>>>
>>>> On Mon, 29 Apr 2024 at 00:25, Michael Balzer <dexter at expeedo.de> wrote:
>>>>
>>>>> Michael,
>>>>>
>>>>> forwarding the Twizy logs to you directly, as they contain the user
>>>>> location.
>>>>>
>>>>> He has just verified it's the new version, he says the module stops
>>>>> responding as soon as he turns on the Twizy.
>>>>>
>>>>> His description of his actions is a bit ambiguous, and it seems he
>>>>> didn't enable logging to the file persistently.
>>>>>
>>>>> According to the server, these were the version boot times:
>>>>>
>>>>> 2024-04-28 15:30:56    0    3.3.004-32-g125e0841/ota_0/edge (build idf
>>>>> v3.3.4-849-g6e214dc335 Apr 26 2024 19:16:50)
>>>>> 2024-04-28 16:21:08    0    3.3.004-32-g125e0841/ota_0/edge (build idf
>>>>> v3.3.4-849-g6e214dc335 Apr 26 2024 19:16:50)
>>>>> 2024-04-28 16:38:33    0    3.3.004-63-gf3595561/ota_1/edge (build idf
>>>>> v3.3.4-849-g6e214dc335 Apr 27 2024 07:44:50)
>>>>> 2024-04-28 16:40:57    0    3.3.004-63-gf3595561/ota_1/edge (build idf
>>>>> v3.3.4-849-g6e214dc335 Apr 27 2024 07:44:50)
>>>>> 2024-04-28 16:43:14    0    3.3.004-63-gf3595561/ota_1/edge (build idf
>>>>> v3.3.4-849-g6e214dc335 Apr 27 2024 07:44:50)
>>>>> 2024-04-28 16:46:39    0    3.3.004-63-gf3595561/ota_1/edge (build idf
>>>>> v3.3.4-849-g6e214dc335 Apr 27 2024 07:44:50)
>>>>> 2024-04-28 16:54:44    0    3.3.004-32-g125e0841/ota_0/edge (build idf
>>>>> v3.3.4-849-g6e214dc335 Apr 26 2024 19:16:50)
>>>>>
>>>>> Attached is also his crash debug log -- a2ll doesn't tell much about
>>>>> what happened, but maybe you get an idea from this.
>>>>>
>>>>> After the boot at 16:46, there are immediately lots of these messages:
>>>>>
>>>>> 2024-04-28 16:46:33.792 CEST D (39042) vehicle-poll: Poller: Queue
>>>>> PollerFrame()
>>>>> 2024-04-28 16:46:33.792 CEST D (39042) vehicle-poll: [1]Poller:
>>>>> Incoming - giving up
>>>>>
>>>>> Regards,
>>>>> Michael
>>>>>
>>>>>
>>>>> Am 28.04.24 um 16:44 schrieb Michael Geddes:
>>>>>
>>>>> Ah. OK.
>>>>>
>>>>> I could try to fix the vin thing using the new way of doing it and get
>>>>> rid of a semaphore?
>>>>> It would at least identify the problem possibly?
>>>>>
>>>>> Michael
>>>>>
>>>>>
>>>>> On Sun, 28 Apr 2024, 22:32 Michael Balzer via OvmsDev, <
>>>>> ovmsdev at lists.openvehicles.com> wrote:
>>>>>
>>>>>> Not sure if that's the problem, but I've found a different behaviour
>>>>>> with the new PollSetState() implementation.
>>>>>>
>>>>>> The old version only did anything if the new state actually was
>>>>>> different from the previous one. The Twizy relies on this behaviour,
>>>>>> calling PollSetState() from the per second ticker (see
>>>>>> OvmsVehicleRenaultTwizy::ObdTicker1).
>>>>>>
>>>>>> The new implementation apparently always sends the PollState command
>>>>>> to the task, and that in turn always at least locks the poller mutex. Not
>>>>>> sure if/how that could cause the observed issues, but it definitely adds
>>>>>> quite some (unnecessary?) lock/unlock operations.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Am 28.04.24 um 16:05 schrieb Michael Balzer via OvmsDev:
>>>>>>
>>>>>> The Twizy uses the poller to query the VIN (once) and DTCs (every 10
>>>>>> seconds while driving), see rt_obd2.cpp.
>>>>>>
>>>>>> It also has its own version of the OBD single request
>>>>>> (OvmsVehicleRenaultTwizy::ObdRequest), which was the precursor for the
>>>>>> generalized version. This is used by custom/user Twizy plugins and scripts
>>>>>> to access ECU internals.
>>>>>>
>>>>>> The Twizy doesn't use IncomingPollRxFrame, but the Twizy's
>>>>>> IncomingPollReply handler will log any poll responses it doesn't know
>>>>>> about, so that could lead to a lot of log output if something goes wrong
>>>>>> there.
>>>>>>
>>>>>>
>>>>>> Am 28.04.24 um 15:49 schrieb Michael Geddes via OvmsDev:
>>>>>>
>>>>>> AFAICT the twizzy doesn't use the poller list at all.  So is it
>>>>>> missing a call-back or something??
>>>>>>
>>>>>> I can see a potential problem with IncomingPollRxFrame being called
>>>>>> twice as much as it should be but only when there is a poll list.  Maybe
>>>>>> commenting out this would do it.  (I can find another away to get this
>>>>>> called on the thread I want).  This might be the problem with the smarted
>>>>>>
>>>>>> void OvmsVehicle::OvmsVehicleSignal::IncomingPollRxFrame(canbus* bus,
>>>>>> CAN_frame_t *frame, bool success)
>>>>>>   {
>>>>>>   //if (Ready())
>>>>>>   //  m_parent->IncomingPollRxFrame(frame, success);
>>>>>>   }
>>>>>>
>>>>>> //.
>>>>>>
>>>>>>
>>>>>> On Sun, 28 Apr 2024 at 21:10, Michael Balzer via OvmsDev <
>>>>>> ovmsdev at lists.openvehicles.com> wrote:
>>>>>>
>>>>>>> There may also be an issue with the Renault Twizy, I've received a
>>>>>>> report of a user who is using the edge builds, that the latest build
>>>>>>> wouldn't work.
>>>>>>>
>>>>>>> He reports all kinds of errors and warnings signaled by the car
>>>>>>> during driving, and switching back to the previous build fixed the issues.
>>>>>>>
>>>>>>> I've asked him to provide a debug log excerpt if possible.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Michael
>>>>>>>
>>>>>>>
>>>>>>> Am 28.04.24 um 14:29 schrieb Michael Geddes via OvmsDev:
>>>>>>>
>>>>>>> OK. That's bad.
>>>>>>>
>>>>>>> Does the reading work in general?
>>>>>>>
>>>>>>> Is it just the writing commands?
>>>>>>>
>>>>>>> Raise a ticket on github and tag me in and we can address it that
>>>>>>> way.
>>>>>>>
>>>>>>> Michael
>>>>>>>
>>>>>>> On Sun, 28 Apr 2024, 19:49 Thomas Heuer via OvmsDev, <
>>>>>>> ovmsdev at lists.openvehicles.com> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> The new poller code doesn't seem to work properly with the smarted.
>>>>>>>>
>>>>>>>> D (218831) vehicle-poll: [1]PollerNextTick(PRI): cycle complete for
>>>>>>>> ticker=215
>>>>>>>>
>>>>>>>> V (218831) vehicle-poll: Standard Poll Series: List reset
>>>>>>>>
>>>>>>>> D (218831) vehicle-poll:
>>>>>>>> PollSeriesList::NextPollEntry[!v.standard]: ReachedEnd
>>>>>>>>
>>>>>>>> V (218831) vehicle-poll: [1]PollerSend: Poller Reached End
>>>>>>>>
>>>>>>>> D (219691) vehicle-poll: Poller: Queue PollerFrame()
>>>>>>>>
>>>>>>>> D (219691) vehicle-poll: Poller: Queue PollerFrame()
>>>>>>>>
>>>>>>>> V (219691) vehicle-poll: Pollers: FrameRx(bus=2)
>>>>>>>>
>>>>>>>> D (219691) vehicle-poll: Poller: Queue PollerFrame()
>>>>>>>>
>>>>>>>> V (219691) vehicle-poll: Pollers: FrameRx(bus=2)
>>>>>>>>
>>>>>>>> V (219691) vehicle-poll: Pollers: FrameRx(bus=2)
>>>>>>>>
>>>>>>>> D (219691) vehicle-poll: Poller: Queue PollerFrame()
>>>>>>>>
>>>>>>>> *OVMS#* unlock 22
>>>>>>>> Vehicle unlocked
>>>>>>>>
>>>>>>>> V (219691) vehicle-poll: Pollers: FrameRx(bus=2)
>>>>>>>>
>>>>>>>> D (219691) vehicle-poll: Poller: Queue PollerFrame()
>>>>>>>>
>>>>>>>> V (219691) vehicle-poll: Pollers: FrameRx(bus=2)
>>>>>>>>
>>>>>>>> D (219701) vehicle-poll: Poller: Queue PollerFrame()
>>>>>>>>
>>>>>>>> V (219701) vehicle-poll: Pollers: FrameRx(bus=2)
>>>>>>>>
>>>>>>>> D (219701) vehicle-poll: Poller: Queue PollerFrame()
>>>>>>>>
>>>>>>>> V (219701) vehicle-poll: Pollers: FrameRx(bus=2)
>>>>>>>>
>>>>>>>> D (219701) vehicle-poll: Poller: Queue PollerFrame()
>>>>>>>>
>>>>>>>> V (219701) vehicle-poll: Pollers: FrameRx(bus=2)
>>>>>>>>
>>>>>>>> D (219701) vehicle-poll: Poller: Queue PollerFrame()
>>>>>>>>
>>>>>>>> V (219701) vehicle-poll: Pollers: FrameRx(bus=2)
>>>>>>>>
>>>>>>>> D (219701) vehicle-poll: Poller: Queue PollerFrame()
>>>>>>>>
>>>>>>>> V (219701) vehicle-poll: Pollers: FrameRx(bus=2)
>>>>>>>>
>>>>>>>> D (219701) vehicle-poll: Poller: Queue PollerFrame()
>>>>>>>>
>>>>>>>> V (219701) vehicle-poll: Pollers: FrameRx(bus=2)
>>>>>>>>
>>>>>>>> D (219701) vehicle-poll: Poller: Queue PollerFrame()
>>>>>>>>
>>>>>>>> V (219701) vehicle-poll: Pollers: FrameRx(bus=2)
>>>>>>>>
>>>>>>>> D (219701) vehicle-poll: Poller: Queue PollerFrame()
>>>>>>>>
>>>>>>>> V (219701) vehicle-poll: Pollers: FrameRx(bus=2)
>>>>>>>>
>>>>>>>> D (219701) vehicle-poll: Poller: Queue PollerFrame()
>>>>>>>>
>>>>>>>> V (219701) vehicle-poll: Pollers: FrameRx(bus=2)
>>>>>>>>
>>>>>>>> D (219701) vehicle-poll: Poller: Queue PollerFrame()
>>>>>>>>
>>>>>>>> V (219701) vehicle-poll: Pollers: FrameRx(bus=2)
>>>>>>>>
>>>>>>>> D (219701) vehicle-poll: Poller: Queue PollerFrame()
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> *Von:* OvmsDev <ovmsdev-bounces at lists.openvehicles.com> *Im
>>>>>>>> Auftrag von *Michael Geddes via OvmsDev
>>>>>>>> *Gesendet:* Sonntag, 28. April 2024 12:27
>>>>>>>> *An:* OVMS Developers <ovmsdev at lists.openvehicles.com>
>>>>>>>> *Cc:* Michael Geddes <frog at bunyip.wheelycreek.net>
>>>>>>>> *Betreff:* [Ovmsdev] OVMS Poller module/singleton
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Hey all,
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> The poller singleton code that I've been working on for over a year
>>>>>>>> now is merged in. (Thanks Michael for expediting the final step).
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> This includes separate multi-frame states per bus and multiple poll
>>>>>>>> lists as well as non-blocking one off queries. As well as more 'states'.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> I have included some programming documentation in the change but am
>>>>>>>> happy to supply more if needed.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> The ioniq 5 code has some examples of how it can be used. Some
>>>>>>>> examples are:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> * grabbing the vin as a one shot without blocking
>>>>>>>>
>>>>>>>> * having a short list of queries that are polled quickly for
>>>>>>>> obd2ecu (this also demonstrates using a shorter frame break value and then
>>>>>>>> a break after successful a response)
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Have a play please!
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Also interested in hearing what user tools might be worth looking
>>>>>>>> at next for the poller object.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> //.ichael G.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> OvmsDev mailing list
>>>>>>>> OvmsDev at lists.openvehicles.com
>>>>>>>> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
>>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> OvmsDev mailing listOvmsDev at lists.openvehicles.comhttp://lists.openvehicles.com/mailman/listinfo/ovmsdev
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
>>>>>>> Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> OvmsDev mailing list
>>>>>>> OvmsDev at lists.openvehicles.com
>>>>>>> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> OvmsDev mailing listOvmsDev at lists.openvehicles.comhttp://lists.openvehicles.com/mailman/listinfo/ovmsdev
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
>>>>>> Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> OvmsDev mailing listOvmsDev at lists.openvehicles.comhttp://lists.openvehicles.com/mailman/listinfo/ovmsdev
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
>>>>>> Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
>>>>>>
>>>>>> _______________________________________________
>>>>>> OvmsDev mailing list
>>>>>> OvmsDev at lists.openvehicles.com
>>>>>> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
>>>>>>
>>>>>
>>>>> --
>>>>> Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
>>>>> Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
>>>>>
>>>>>
>>> --
>>> Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
>>> Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
>>>
>>>
>> --
>> Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
>> Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
>>
>> _______________________________________________
>> OvmsDev mailing list
>> OvmsDev at lists.openvehicles.com
>> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
>>
>
> --
> Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
> Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
>
> _______________________________________________
> OvmsDev mailing list
> OvmsDev at lists.openvehicles.com
> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvehicles.com/pipermail/ovmsdev/attachments/20240502/cace6d35/attachment-0001.htm>