[Ovmsdev] OBD poller module shell proposal
Michael Geddes
frog at bunyip.wheelycreek.net
Thu Mar 21 17:17:13 HKT 2024
I've pushed up my working tree here:
https://github.com/frogonwheels/Open-Vehicle-Monitoring-System-3/tree/new-poller
partly for feedback, and partly because I wanted it backed up!
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.
Of interest is really the final components/poller/src/vehicle_poller*
files and how that has all come together.. I've added some preliminary
documentation for that which I'm quite happy to accept any critiques or
requests for specific information!
The Duktape metrics 'stale, age' methods are probably ready for a p/r
as-is.
//.ichael
On Sun, 17 Mar 2024 at 16:58, Michael Geddes <frog at bunyip.wheelycreek.net>
wrote:
> 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/20240321/0e11b127/attachment.htm>
More information about the OvmsDev
mailing list