[Ovmsdev] OBD poller module shell proposal
Michael Geddes
frog at bunyip.wheelycreek.net
Sun Mar 17 16:58:05 HKT 2024
Thanks Michael,
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?
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.
In my example the event notification looks like this:
void PollRunFinished(canbus *bus)
{
m_runfinished_callback.Call(
[bus](const std::string &name, const PollCallback &cb)
{
cb(bus, nullptr);
});
}
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.
Thoughts?
//.ichael
--------------8< -----------------------------------
/* Call-back register.
* The list does not shrink which is fine for this use-case.
* Can be made inexpensively threadsafe/re-entrant safe.
*/
template <typename FN>
class OvmsCallBackRegister
{
private:
class CallbackEntry
{
public:
CallbackEntry(const std::string &caller, FN callback)
{
m_name = caller;
m_callback = callback;
}
~CallbackEntry() {}
public:
std::string m_name;
FN m_callback;
};
typedef std::forward_list<CallbackEntry> callbacklist_t;
callbacklist_t m_list;
public:
~OvmsCallBackRegister()
{
}
void Register(const std::string &nametag, FN callback)
{
// Replace
for (auto it = m_list.begin(); it != m_list.end(); ++it)
{
if ((*it).m_name == nametag)
{
(*it).m_callback = callback;
return;
}
}
if (!callback)
return;
for (auto it = m_list.begin(); it != m_list.end(); ++it)
{
if (!(*it).m_callback)
{
CallbackEntry &entry = *it;
entry.m_name = nametag;
entry.m_callback = callback;
return;
}
}
m_list.push_front(CallbackEntry(nametag, callback));
}
void Deregister(const std::string &nametag)
{
Register(nametag, nullptr);
}
typedef std::function<void (const std::string &nametag, FN callback)>
visit_fn_t;
void Call(visit_fn_t visit)
{
for (auto it = m_list.begin(); it != m_list.end(); ++it)
{
const CallbackEntry &entry = *it;
if (entry.m_callback)
visit(entry.m_name, entry.m_callback);
}
}
};
On Thu, 14 Mar 2024 at 12:08, Mark Webb-Johnson <mark at webb-johnson.net>
wrote:
> Michael,
>
> 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).
>
> 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).
>
> Regards, Mark
>
> On 14 Mar 2024, at 11:01 AM, Michael Geddes <frog at bunyip.wheelycreek.net>
> wrote:
>
> Thanks Michael, Mark,
>
> Sorry for not acknowledging earlier.. this feedback is great; I've just
> been cogitating on the consequences.
>
> 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).
>
> 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..
>
> 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.
>
> 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).
>
> //.ichael
>
>
> On Mon, 11 Mar 2024 at 14:51, Michael Balzer <dexter at expeedo.de> wrote:
>
>> 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.
>>
>> 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).
>>
>> Regards,
>> Michael
>>
>>
>> Am 11.03.24 um 00:51 schrieb Mark Webb-Johnson:
>>
>> Michael,
>>
>> 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).
>>
>> If *only* within vehicle framework, then putting it as a sub-command
>> under ‘vehicle’ seems sensible.
>>
>> If more general purpose, then perhaps look at ‘copen’ (component/canopen)
>> as an example.
>>
>> Regards, Mark.
>>
>> On 10 Mar 2024, at 7:25 AM, Michael Geddes <frog at bunyip.wheelycreek.net>
>> <frog at bunyip.wheelycreek.net> wrote:
>>
>> Hi all,
>>
>> 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).
>> My question is not so much about the functionality and status
>> information, but about the location of the *poller* subcommand. (See
>> below).
>>
>> Should 'vehicle' be exclusively for switching the vehicle type? Should
>> the 'poller' command be top-level? Under obdii?
>>
>> Thoughts welcome.
>> If you do vehicle poller pause then the last line reads 'Vehicle OBD
>> Polling is paused'
>> //.
>> -------8<----------------------------------------
>>
>> *OVMS# vehicle ?*
>> Usage: vehicle [list|module|poller|status]
>> list Show list of available vehicle modules
>> module Set (or clear) vehicle module
>> poller OBD polling status
>> status Show vehicle module status
>> *OVMS# vehicle poller ?*
>> Usage: vehicle poller [pause|resume]
>> pause Pause OBD Polling
>> resume Resume OBD Polling
>> *OVMS# vehicle poller*
>> OBD Polling running on bus 1 with an active list
>> Time between polling ticks is 1000ms with 1 secondary sub-ticks
>> Last poll command received 1s (ticks) ago.
>> Vehicle OBD Polling is running.
>> _______________________________________________
>> OvmsDev mailing list
>> OvmsDev at lists.openvehicles.com
>> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
>>
>>
>>
>> _______________________________________________
>> OvmsDev mailing listOvmsDev at lists.openvehicles.comhttp://lists.openvehicles.com/mailman/listinfo/ovmsdev
>>
>>
>> --
>> 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
>>
> _______________________________________________
> OvmsDev mailing list
> OvmsDev at lists.openvehicles.com
> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
>
>
> _______________________________________________
> 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/20240317/c628ee80/attachment.htm>
More information about the OvmsDev
mailing list