[Ovmsdev] P/R 859 - Moving polling to separate thread & More polling ideas
Michael Geddes
frog at bunyip.wheelycreek.net
Sat Apr 8 10:07:21 HKT 2023
Thanks Michael,
I'm not sure what exactly is wrong with what I have done, it is definitely
a step on a path, rather than the destination though.
This is a minimal change trying not to upset things too much. Do you think
what I've done changes existing behaviour? It shouldn't, but I might have
missed something. About all it does is assures that the Ticker threads
doesn't impact on the polling. Yes, it also adds the ability to vary the
primary Poll sending frequency, and adds a secondary tick with minimal (so
far) consequences. It does put poll transmits onto the RXTask() thread
rather than the Ticker1() thread and I'm now seeing this might be a
temporary arrangement, where the goal would be to have a Command thread or
something per CAN Bus which would handle the poll ticks, and where the
RXTask() thread could send updates (like CanOpen).
I know I'm making some assumptions with threads and setting variables to 0
- It would be good if there were some 'Interlocked' getter/setter/increment
functions (which I'm used to from windows) - but I'm guessing any
implementation would require a critical section anyway, unless I'm missing
something.
This implementation _does_ allow the vehicle instance to
conditionally/dynamically set a faster poll, and to have a subdivided
secondary tick. The effect of this secondary tick is small for now,
relating more to keeping failed sends from halting the queue too much, (but
I have changes that would allow a small poll list to be added that can
re-send on Secondary ticks if throttling allows, without preventing moving
forward on the primary tick).
I have an ioniq 5 specific commit to speed things up setting 2x 400ms
secondary ticks per primary tick for when the Obdecu is enabled. As per
your request, I have not pushed this up as it is specific to the Ioniq5.
Examples of changing the ticker frequency:
PollSetTicker(400, 2); // Primary every 800ms followed by 1 Secondary
tick at 400ms
PollSetTicker(200,4); // Primary every 800ms followed by 3 secondary
ticks at 200ms.
One thing I discovered about the I5 was that I needed to hold off on a
faster poll till the general car initialisation happened. Otherwise it
seemed to block some of the results of the start up checks causing errors
on the console.
I've had a look at the Can Open code as suggested, and I'm now starting to
get the idea how the Rx thread and a command thread would play off each
other.
I would like to have the message queue as a goal further ahead, and move
things gradually to a point where this is easier. for eg
- limit access to the exposed protected variables (eg, m_poll_plist and
m_poll_curr moved to private) - I already have a commit for this
- Move access to any changes to the list handling behind a small number
of functions
- Separating the ISOTP/ISOVW into a separate class (along with many of
m_poll_*) (separate files / separate directories?? )
- Move the Poll List variables (Like m_poll_ticker) to a separate class
- Not allowing a different target Bus on the poll lists; have a separate
list per Bus (after moving all that to a separate class above).
I have a mechanism ready-to-go to handle multiple poll-lists for a given
bus rather than just one we allow for the moment.
//.ichael
On Fri, 7 Apr 2023, 2:38 pm Michael Balzer, <dexter at expeedo.de> wrote:
> Michael,
>
> I'm sorry I currently don't have the time to support you properly. I had a
> look at your PR some days ago, saw some issues and got the impression
> something must be missing either from your PR or in my understanding of
> your goal with that PR – it seemed the changes now wouldn't have any effect
> on poll frequency at all, which I assumed was the initial goal. I didn't
> have the time to do a proper analysis.
>
> Regarding moving the poller to a job worker/client model, that's very
> welcome. I recommend taking a look at my CANopen implementation, which
> implements that model and provides workers per bus, asynchronous and
> synchronous client APIs and a general shell interface. While CANopen is
> quite different from ISOTP, the basic principles are the same.
>
> CANopen usage:
> https://docs.openvehicles.com/en/latest/components/canopen/docs/index.html
>
> That was my first larger FreeRTOS application, so may be improvable, but
> it works nicely (currently only used by the Twizy adapter).
>
> Regards,
> Michael
>
>
> Am 06.04.23 um 11:45 schrieb Michael Geddes:
>
> This is an example of where I'm headed. This creates and activates a
> Once-off poll entry that doesn't block (and removes itself from the poll
> list once it is done).
> A success will call Incoming_Full with a std::string buffer and a fail
> calls Incoming_Fail.
>
> bool OvmsHyundaiIoniqEv::PollRequestVIN()
> {
> if (!StdMetrics.ms_v_env_awake->AsBool()) {
> ESP_LOGV(TAG, "PollRequestVIN: Not Awake Request not sent");
> return false;
> }
> auto poll_entry = std::shared_ptr<OvmsVehicle::OnceOffPoll>(
> new OvmsVehicle::OnceOffPoll(m_can1,
> std::bind(&OvmsHyundaiIoniqEv::Incoming_Full, this, _1, _2, _3,
> _4, _5, _6),
> std::bind(&OvmsHyundaiIoniqEv::Incoming_Fail, this, _1, _2, _3,
> _4, _5, _6),
> VEHICLE_OBD_BROADCAST_MODULE_TX, VEHICLE_OBD_BROADCAST_MODULE_RX,
> VEHICLE_POLL_TYPE_OBDIIVEHICLE, 2,
> ISOTP_STD, 0, 3/*retries*/ ));
> PollRequest("!xiq.vin", poll_entry);
> return true;
> }
>
>
> On Tue, 4 Apr 2023 at 07:13, Michael Geddes <frog at bunyip.wheelycreek.net>
> wrote:
>
>> Hi all,
>> I was after some feedback on
>> https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/pull/859
>>
>> which moves the ISOTP polling into what is currently the ISOTP response
>> thread.
>>
>> This is part of a larger rework I have been doing to add flexibility to
>> the loop and to move the poll-loop operations behind a clean interface.
>>
>> I'm interested in feedback on the mechanism in the p/r directly, but I'm
>> also happy to provide the usage examples I have (which would be the
>> follow-up commits for Ioniq 5).
>>
>> I'm also interested in other people's thoughts on where they see what
>> improvements might be had to the poll-list mechanism.
>> Already Done:
>> * Clean up the blocking poll mechanism
>> * Allow for multiple concurrent poll lists (still executed sequentially)
>> * Add once-off non-blocking poll mechanism
>> * Add a poll-list that allows repeating more-frequently than the
>> primary poll
>>
>> Ideas:
>> * Add a user-configurable poller that could possibly utilise the DBC
>> framework (on top of the existing vehicle framework) to allow
>> exploring/experimenting with both passive data as well as polled data.
>> (Given that the DBC framework already does the heavy lifting of
>> interpreting data - adding a poll request to that seems to be a nice way of
>> going)
>> * ??? Have a poll thread per bus ???
>>
>> //.ichael
>>
>>
>>
> --
> 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/20230408/0a01b5ef/attachment-0001.htm>
More information about the OvmsDev
mailing list