<div dir="ltr">Thanks Michael,<div>I'll definitely add the config - pretty much sorted out the singleton (except for the change of directory). Just wondering if I should rebase this change down earlier or just keep it as the 'last change' that is making the final break. It might be a better transition that way?</div><div><br></div><div>Anyway have a look at this little class below; we have a bunch of different implementations for an event register and came up with the OvmsCallBackRegister below.</div><div><br></div><div>In my example the event notification looks like this:<br> void PollRunFinished(canbus *bus)<br> {<br></div><div> m_runfinished_callback.Call(<br> [bus](const std::string &name, const PollCallback &cb)<br> {<br> cb(bus, nullptr);<br> });<br></div><div> }</div><div><br></div><div>I could possibly but it in main/ovms_utils.h ?? It could also use a std:map to implement it, though I think we save space this way and tbh the use case wouldn't be doing a lot of Register and Unregister which would be the slowest operations... I've made it not shrink the list size .... but again, not that important in the context.<br></div><div><br></div><div>Thoughts?</div><div><br></div><div>//.ichael</div><div>--------------8< -----------------------------------</div><div><div><font face="monospace">/* Call-back register.<br> * The list does not shrink which is fine for this use-case.<br> * Can be made inexpensively threadsafe/re-entrant safe.<br> */<br>template <typename FN><br>class OvmsCallBackRegister<br> {<br> private:<br> class CallbackEntry<br> {<br> public:<br> CallbackEntry(const std::string &caller, FN callback)<br> {<br> m_name = caller;<br> m_callback = callback;<br> }<br> ~CallbackEntry() {}<br> public:<br> std::string m_name;<br> FN m_callback;<br> };<br> typedef std::forward_list<CallbackEntry> callbacklist_t;<br> callbacklist_t m_list;<br> public:<br> ~OvmsCallBackRegister()<br> {<br> }<br> void Register(const std::string &nametag, FN callback)<br> {<br> // Replace<br> for (auto it = m_list.begin(); it != m_list.end(); ++it)<br> {<br> if ((*it).m_name == nametag)<br> {<br> (*it).m_callback = callback;<br> return;<br> }<br> }<br> if (!callback)<br> return;<br> for (auto it = m_list.begin(); it != m_list.end(); ++it)<br> {<br> if (!(*it).m_callback)<br> {<br> CallbackEntry &entry = *it;<br> entry.m_name = nametag;<br> entry.m_callback = callback;<br> return;<br> }<br> }<br> m_list.push_front(CallbackEntry(nametag, callback));<br> }<br> void Deregister(const std::string &nametag)<br> {<br> Register(nametag, nullptr);<br> }<br> typedef std::function<void (const std::string &nametag, FN callback)> visit_fn_t;<br> void Call(visit_fn_t visit)<br> {<br> for (auto it = m_list.begin(); it != m_list.end(); ++it)<br> {<br> const CallbackEntry &entry = *it;<br> if (entry.m_callback)<br> visit(entry.m_name, entry.m_callback);<br> }<br> }<br> };</font><br></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 14 Mar 2024 at 12:08, Mark Webb-Johnson <<a href="mailto:mark@webb-johnson.net">mark@webb-johnson.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>Michael,<div><br></div><div>I suggest that if it is a separate component then better to move it to it’s own component directory (just as canopen is done).</div><div><br></div><div>For completeness, I suggest it would also be good to include a CONFIG_OVMS_COMP_* sdkconfig (default: yes), and put that as a requirement for your component (as well as for any vehicle doing polling, I guess).</div><div><br></div><div>Regards, Mark<br id="m_-8888443040235954155lineBreakAtBeginningOfMessage"><div><br><blockquote type="cite"><div>On 14 Mar 2024, at 11:01 AM, Michael Geddes <<a href="mailto:frog@bunyip.wheelycreek.net" target="_blank">frog@bunyip.wheelycreek.net</a>> wrote:</div><br><div><div dir="ltr"><div>Thanks Michael, Mark,</div><div><br></div><div>Sorry for not acknowledging earlier.. this feedback is great; I've just been cogitating on the consequences. </div><div><br></div>I still have the Poller hanging onto the vehicle by a thread so I should just cut the thread making the Poller a separate singleton (it's still embedded in the vehicle class for now with a small interface joining them).<div><br></div><div>If I do, does it need to get moved to a new directory or can it stay in the vehicle/ directory? The file vehicle_poller.cpp (and the _isotp and vwtp parts to it) are still pretty much as they were with only a change in class name..<br><div><br></div><div>I think I just need the poller to get its own values of m_can1 etc and provide a different way of getting the feedback results.</div><div><br></div><div>I also need to make sure I'm not cutting off the 'vehicle' class' access to non-solicited messages (ie stuff that is just on the bus).</div><div><br></div><div>//.ichael</div><div><br></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 11 Mar 2024 at 14:51, Michael Balzer <<a href="mailto:dexter@expeedo.de" target="_blank">dexter@expeedo.de</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"><u></u>
<div>
Actually, separating the poller from the vehicle was part of the
plan of reworking it into a job/worker architecture. I see no reason
the generalized poller would need to remain coupled to the vehicle.<br>
<br>
That's why I placed the OBD single request command in the "obdii"
hierarchy (although a more proper naming would have been e.g.
"isotp", but changing the name or having both would confuse users --
and meanwhile the poller also supports a non-ISO TP variant).<br>
<br>
Regards,<br>
Michael<br>
<br>
<br>
<div>Am 11.03.24 um 00:51 schrieb Mark
Webb-Johnson:<br>
</div>
<blockquote type="cite">
Michael,
<div><br>
</div>
<div>It depends on whether the poller can *only* be used in the
vehicle class or if it is a framework all by itself (for example
with commands to manually poll specific PIDs, etc).</div>
<div><br>
</div>
<div>If *only* within vehicle framework, then putting it as a
sub-command under ‘vehicle’ seems sensible.</div>
<div><br>
</div>
<div>If more general purpose, then perhaps look at ‘copen’
(component/canopen) as an example.</div>
<div><br>
</div>
<div>Regards, Mark.<br id="m_-8888443040235954155m_393746923709381916lineBreakAtBeginningOfMessage">
<div><br>
<blockquote type="cite">
<div>On 10 Mar 2024, at 7:25 AM, Michael Geddes
<a href="mailto:frog@bunyip.wheelycreek.net" target="_blank"><frog@bunyip.wheelycreek.net></a> wrote:</div>
<br>
<div>
<div dir="ltr">
<div>Hi all,</div>
<div><br>
</div>
<div>I know some of this (especially for the status)
functionality is predicated on code that's not gone up
yet - however this is allowing 'pause' and 'resume' of
the poller (which has been merged).</div>
<div>My question is not so much about the
functionality and status information, but about the
location of the <b>poller</b> subcommand. (See
below). </div>
<div><br>
</div>
<div>Should 'vehicle' be exclusively for switching the
vehicle type? Should the 'poller' command be
top-level? Under obdii? </div>
<div><br>
</div>
<div>Thoughts welcome.</div>
<div>If you do vehicle poller pause then the last line
reads 'Vehicle OBD Polling is paused'</div>
<div>//.</div>
<div>-------8<----------------------------------------</div>
<br>
<b>OVMS# vehicle ?</b><br>
Usage: vehicle [list|module|poller|status]<br>
list Show list of available vehicle
modules<br>
module Set (or clear) vehicle module<br>
poller OBD polling status<br>
status Show vehicle module status<br>
<b>OVMS# vehicle poller ?</b><br>
Usage: vehicle poller [pause|resume]<br>
pause Pause OBD Polling<br>
resume Resume OBD Polling<b><br>
</b>
<div><b>OVMS# vehicle poller</b><br>
OBD Polling running on bus 1 with an active list<br>
Time between polling ticks is 1000ms with 1 secondary
sub-ticks<br>
Last poll command received 1s (ticks) ago.<br>
Vehicle OBD Polling is running.<br>
</div>
</div>
_______________________________________________<br>
OvmsDev mailing list<br>
<a href="mailto:OvmsDev@lists.openvehicles.com" target="_blank">OvmsDev@lists.openvehicles.com</a><br>
<a href="http://lists.openvehicles.com/mailman/listinfo/ovmsdev" target="_blank">http://lists.openvehicles.com/mailman/listinfo/ovmsdev</a><br>
</div>
</blockquote>
</div>
<br>
</div>
<br>
<fieldset></fieldset>
<pre>_______________________________________________
OvmsDev mailing list
<a href="mailto:OvmsDev@lists.openvehicles.com" target="_blank">OvmsDev@lists.openvehicles.com</a>
<a href="http://lists.openvehicles.com/mailman/listinfo/ovmsdev" target="_blank">http://lists.openvehicles.com/mailman/listinfo/ovmsdev</a>
</pre>
</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" target="_blank">OvmsDev@lists.openvehicles.com</a><br>
<a href="http://lists.openvehicles.com/mailman/listinfo/ovmsdev" rel="noreferrer" target="_blank">http://lists.openvehicles.com/mailman/listinfo/ovmsdev</a><br>
</blockquote></div>
_______________________________________________<br>OvmsDev mailing list<br><a href="mailto:OvmsDev@lists.openvehicles.com" target="_blank">OvmsDev@lists.openvehicles.com</a><br><a href="http://lists.openvehicles.com/mailman/listinfo/ovmsdev" target="_blank">http://lists.openvehicles.com/mailman/listinfo/ovmsdev</a><br></div></blockquote></div><br></div></div>_______________________________________________<br>
OvmsDev mailing list<br>
<a href="mailto:OvmsDev@lists.openvehicles.com" target="_blank">OvmsDev@lists.openvehicles.com</a><br>
<a href="http://lists.openvehicles.com/mailman/listinfo/ovmsdev" rel="noreferrer" target="_blank">http://lists.openvehicles.com/mailman/listinfo/ovmsdev</a><br>
</blockquote></div>