<div dir="ltr"><div dir="auto"><div dir="auto"><div dir="auto">Thanks Michael,<br></div><div dir="auto"><br></div><div dir="auto">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. </div><div dir="auto"><br></div><div dir="auto">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).</div><div dir="auto"><br></div><div dir="auto"> 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.</div><div dir="auto"><br><div dir="auto">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).</div><div dir="auto"> </div><div dir="auto">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:</div><div dir="auto"><font face="monospace"><br></font></div><font face="monospace"> PollSetTicker(400, 2); // Primary every 800ms followed by 1 Secondary tick at 400ms</font></div><div dir="auto"><font face="monospace"> PollSetTicker(200,4); // Primary every 800ms followed by 3 secondary ticks at 200ms.<br></font><div dir="auto"><font face="monospace"> </font></div><div dir="auto">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. <br></div></div></div><div dir="auto"> </div><div dir="auto">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.</div><div dir="auto"> </div>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 </div><div><ul><li>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</li><li>Move access to any changes to the list handling behind a small number of functions </li><li>Separating the ISOTP/ISOVW into a separate class (along with many of m_poll_*) (separate files / separate directories?? )</li><li>Move the Poll List variables (Like m_poll_ticker) to a separate class </li><li>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).</li></ul></div><div dir="auto"><div>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.</div><div><br></div><div>//.ichael<br></div></div><div dir="auto"><br><div class="gmail_quote" dir="auto"><div dir="ltr" class="gmail_attr">On Fri, 7 Apr 2023, 2:38 pm Michael Balzer, <<a href="mailto:dexter@expeedo.de" rel="noreferrer noreferrer noreferrer" target="_blank">dexter@expeedo.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
Michael,<br>
<br>
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.<br>
<br>
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.<br>
<br>
CANopen usage:
<a href="https://docs.openvehicles.com/en/latest/components/canopen/docs/index.html" rel="noreferrer noreferrer noreferrer noreferrer" target="_blank">https://docs.openvehicles.com/en/latest/components/canopen/docs/index.html</a><br>
<br>
That was my first larger FreeRTOS application, so may be improvable,
but it works nicely (currently only used by the Twizy adapter).<br>
<br>
Regards,<br>
Michael<br>
<br>
<br>
<div>Am 06.04.23 um 11:45 schrieb Michael
Geddes:<br>
</div>
<blockquote type="cite">
<div dir="ltr">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).
<div>A success will call Incoming_Full with a std::string
buffer and a fail calls Incoming_Fail.</div>
<div><br>
<blockquote style="margin:0 0 0 40px;border:none;padding:0px">
<div><font face="monospace">bool
OvmsHyundaiIoniqEv::PollRequestVIN()</font></div>
<div><font face="monospace">{</font></div>
<div><font face="monospace"> if
(!StdMetrics.ms_v_env_awake->AsBool()) {</font></div>
<div><font face="monospace"> ESP_LOGV(TAG,
"PollRequestVIN: Not Awake Request not sent");</font></div>
<div><font face="monospace"> return false;</font></div>
<div><font face="monospace"> }</font></div>
<div><font face="monospace"> auto poll_entry =
std::shared_ptr<OvmsVehicle::OnceOffPoll>(</font></div>
<div><font face="monospace"> new
OvmsVehicle::OnceOffPoll(m_can1,</font></div>
<div><font face="monospace">
std::bind(&OvmsHyundaiIoniqEv::Incoming_Full, this,
_1, _2, _3, _4, _5, _6),</font></div>
<div><font face="monospace">
std::bind(&OvmsHyundaiIoniqEv::Incoming_Fail, this,
_1, _2, _3, _4, _5, _6),</font></div>
<div><font face="monospace">
VEHICLE_OBD_BROADCAST_MODULE_TX,
VEHICLE_OBD_BROADCAST_MODULE_RX,</font></div>
<div><font face="monospace">
VEHICLE_POLL_TYPE_OBDIIVEHICLE, 2,</font></div>
<div><font face="monospace"> ISOTP_STD, 0,
3/*retries*/ ));</font></div>
<div><font face="monospace"> PollRequest("!xiq.vin",
poll_entry);</font></div>
<div><font face="monospace"> return true;</font></div>
<div><font face="monospace">}</font></div>
</blockquote>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Tue, 4 Apr 2023 at 07:13,
Michael Geddes <<a href="mailto:frog@bunyip.wheelycreek.net" rel="noreferrer noreferrer noreferrer noreferrer" target="_blank">frog@bunyip.wheelycreek.net</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr">
<div>Hi all,<br>
</div>
<div>I was after some feedback on </div>
<a href="https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/pull/859" rel="noreferrer noreferrer noreferrer noreferrer" target="_blank">https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/pull/859</a>
<div>which moves the ISOTP polling into what is currently
the ISOTP response thread.</div>
<div><br>
</div>
<div>
<div>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.<br>
</div>
</div>
<div><br>
</div>
<div>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).</div>
<div><br>
</div>
<div>I'm also interested in other people's thoughts on where
they see what improvements might be had to the poll-list
mechanism.</div>
<div>Already Done:</div>
<div> * Clean up the blocking poll mechanism</div>
<div> * Allow for multiple concurrent poll lists (still
executed sequentially)</div>
<div> * Add once-off non-blocking poll mechanism</div>
<div> * Add a poll-list that allows repeating
more-frequently than the primary poll</div>
<div><br>
</div>
<div>Ideas:</div>
<div> * 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)</div>
<div> * ??? Have a poll thread per bus ???</div>
<div> </div>
<div>//.ichael</div>
<div><br>
</div>
<div><br>
</div>
</div>
</blockquote>
</div>
</blockquote>
<br>
<pre cols="72">--
Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
Fon 02333 / 833 5735 * Handy 0176 / 206 989 26</pre>
</div>
_______________________________________________<br>
OvmsDev mailing list<br>
<a href="mailto:OvmsDev@lists.openvehicles.com" rel="noreferrer noreferrer noreferrer noreferrer" target="_blank">OvmsDev@lists.openvehicles.com</a><br>
<a href="http://lists.openvehicles.com/mailman/listinfo/ovmsdev" rel="noreferrer noreferrer noreferrer noreferrer noreferrer" target="_blank">http://lists.openvehicles.com/mailman/listinfo/ovmsdev</a><br>
</blockquote></div>
</div>
</div>