<div dir="ltr">Yeah wow. So close and yet so far.<div><br></div><div>Fix pushed.</div><div><br></div><div>//.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, 23 Mar 2024 at 21:58, Michael Balzer <<a href="mailto:dexter@expeedo.de">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>
Michael,<br>
<br>
looking forward to test this, but trying to run your branch with the
VW e-Up leads to an early crash boot loop.<br>
<br>
Apparently `OvmsPollers::PollSetPidList()` doesn't check for a NULL
plist:<br>
<br>
<font face="monospace">0x40195b6f is in
OvmsPollers::PollSetPidList(canbus*, OvmsPoller::poll_pid_t
const*, OvmsPoller::VehicleSignal*)
(/home/balzer/esp/Open-Vehicle-Monitoring-System-3/vehicle/OVMS.V3/components/poller/src/vehicle_poller.cpp:1201).<br>
1196 bool hasbus[1+VEHICLE_MAXBUSSES];<br>
1197 for (int i = 0 ; i <= VEHICLE_MAXBUSSES; ++i)<br>
1198 hasbus[i] = false;<br>
1199 <br>
1200 // Check for an Empty list.<br>
<font color="#d83a3a"><b>1201 if (plist->txmoduleid == 0)</b></font><br>
1202 {<br>
1203 plist = nullptr;<br>
1204 ESP_LOGD(TAG, "PollSetPidList - Setting Empty List");<br>
1205 }<br>
0x401a2675 is in OvmsVehicle::PollSetPidList(canbus*,
OvmsPoller::poll_pid_t const*)
(/home/balzer/esp/Open-Vehicle-Monitoring-System-3/vehicle/OVMS.V3/components/vehicle/vehicle.cpp:2278).<br>
2273 return m_pollsignal;<br>
2274 }<br>
2275 void OvmsVehicle::PollSetPidList(canbus* bus, const
OvmsPoller::poll_pid_t* plist)<br>
2276 {<br>
2277 m_poll_bus_default = bus;<br>
2278 MyPollers.PollSetPidList(bus, plist, GetPollerSignal());<br>
2279 }<br>
2280 #endif<br>
2281 <br>
2282 /**<br>
0x4023209e is in OvmsVehicleVWeUp::OBDInit()
(/home/balzer/esp/Open-Vehicle-Monitoring-System-3/vehicle/OVMS.V3/components/vehicle_vweup/src/vweup_obd.cpp:233).<br>
228 obd_state_t previous_state = m_obd_state;<br>
229 m_obd_state = OBDS_Config;<br>
230 <br>
231 if (previous_state != OBDS_Pause)<br>
232 {<br>
<font color="#d83a3a"><b>233 PollSetPidList(m_can1, NULL);</b></font><br>
234 PollSetThrottling(0);<br>
235 PollSetResponseSeparationTime(1);<br>
236 <br>
237 if
(StandardMetrics.ms_v_charge_inprogress->AsBool())</font><br>
<br>
<br>
Regards,<br>
Michael<br>
<br>
<br>
<div>Am 21.03.24 um 10:17 schrieb Michael
Geddes:<br>
</div>
<blockquote type="cite">
<div dir="ltr">I've pushed up my working tree here: <a href="https://github.com/frogonwheels/Open-Vehicle-Monitoring-System-3/tree/new-poller" target="_blank">https://github.com/frogonwheels/Open-Vehicle-Monitoring-System-3/tree/new-poller</a>
<div>partly for feedback, and partly because I wanted it backed
up!<br>
<div><br>
</div>
<div>This incorporates about 10 pull/requests worth of work
which is why I didn't even add it as a draft, so I see this
as taking some number of months to push up.</div>
<div><br>
</div>
<div>Of interest is really the final
components/poller/src/vehicle_poller* files and how that
has all come together.. I've added some preliminary </div>
<div>documentation for that which I'm quite happy to accept
any critiques or requests for specific information!</div>
<div><br>
</div>
<div>The Duktape metrics 'stale, age' methods are probably
ready for a p/r as-is.</div>
<div><br>
</div>
<div>//.ichael</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Sun, 17 Mar 2024 at 16:58,
Michael Geddes <<a href="mailto:frog@bunyip.wheelycreek.net" 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">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" target="_blank">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_-1225568278149805507m_7550483307298702614m_-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">
<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_-1225568278149805507m_7550483307298702614m_-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>
</blockquote>
</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>