[Ovmsdev] OBD poller module shell proposal

Michael Geddes frog at bunyip.wheelycreek.net
Sun Mar 24 21:10:58 HKT 2024


Thanks!

Will get onto it.  Thanks for giving it a go.

Michael

On Sat, 23 Mar 2024, 21:58 Michael Balzer, <dexter at expeedo.de> wrote:

> Michael,
>
> looking forward to test this, but trying to run your branch with the VW
> e-Up leads to an early crash boot loop.
>
> Apparently `OvmsPollers::PollSetPidList()` doesn't check for a NULL plist:
>
> 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).
> 1196      bool hasbus[1+VEHICLE_MAXBUSSES];
> 1197      for (int i = 0 ; i <= VEHICLE_MAXBUSSES; ++i)
> 1198        hasbus[i] = false;
> 1199
> 1200      // Check for an Empty list.
> *1201      if (plist->txmoduleid == 0)*
> 1202        {
> 1203        plist = nullptr;
> 1204        ESP_LOGD(TAG, "PollSetPidList - Setting Empty List");
> 1205        }
> 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).
> 2273      return m_pollsignal;
> 2274      }
> 2275    void OvmsVehicle::PollSetPidList(canbus* bus, const
> OvmsPoller::poll_pid_t* plist)
> 2276      {
> 2277      m_poll_bus_default = bus;
> 2278      MyPollers.PollSetPidList(bus, plist, GetPollerSignal());
> 2279      }
> 2280    #endif
> 2281
> 2282    /**
> 0x4023209e is in OvmsVehicleVWeUp::OBDInit()
> (/home/balzer/esp/Open-Vehicle-Monitoring-System-3/vehicle/OVMS.V3/components/vehicle_vweup/src/vweup_obd.cpp:233).
> 228       obd_state_t previous_state = m_obd_state;
> 229       m_obd_state = OBDS_Config;
> 230
> 231       if (previous_state != OBDS_Pause)
> 232       {
> *233         PollSetPidList(m_can1, NULL);*
> 234         PollSetThrottling(0);
> 235         PollSetResponseSeparationTime(1);
> 236
> 237         if (StandardMetrics.ms_v_charge_inprogress->AsBool())
>
>
> Regards,
> Michael
>
>
> Am 21.03.24 um 10:17 schrieb Michael Geddes:
>
> 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
>>>
>>
> _______________________________________________
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvehicles.com/pipermail/ovmsdev/attachments/20240324/9d01c0fc/attachment-0001.htm>


More information about the OvmsDev mailing list