New subject: OVMS Poller module/singleton

30 Apr 2024 · *OVMS#*

      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@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,
&lt;frog@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
    &lt;dexter@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,
        &lt;ovmsdev@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 &lt;ovmsdev@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, &lt;ovmsdev@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
                    &lt;ovmsdev-bounces@lists.openvehicles.com>
                    *Im Auftrag von *Michael Geddes via OvmsDev
                    *Gesendet:* Sonntag, 28. April 2024 12:27
                    *An:* OVMS Developers
                    &lt;ovmsdev@lists.openvehicles.com>
                    *Cc:* Michael Geddes
                    &lt;frog@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@lists.openvehicles.com
                    http://lists.openvehicles.com/mailman/listinfo/ovmsdev

                _______________________________________________
                OvmsDev mailing list
                OvmsDev@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@lists.openvehicles.com
                http://lists.openvehicles.com/mailman/listinfo/ovmsdev

            _______________________________________________
            OvmsDev mailing list
            OvmsDev@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@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@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

Re: [Ovmsdev] OVMS Poller module/singleton

Links:

tags

participants (7)