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