[Ovmsdev] Fwd: [openvehicles/Open-Vehicle-Monitoring-System-3] f84837: Server V2 & V3: fix registration for notifications...
mark at webb-johnson.net
Tue Nov 19 08:57:10 HKT 2019
Thanks for the clarification. That makes more sense.
I do find the use of a static outside the class kind of kludgy/ugly. What if RegisterReader firstly iterated over the map to see if there was a free slot below m_nextreader? Or change the map to an array? Then we could simply keep the reader ID as a member variable of the class.
Worth it, or not?
> On 18 Nov 2019, at 5:35 PM, Michael Balzer <dexter at expeedo.de> wrote:
> Am 18.11.19 um 02:37 schrieb Mark Webb-Johnson:
>> I don’t think this fix is correct. Seeking clarification of the thinking behind it.
>> Currently, MyOvmsServerV2Reader is initialised to 0. Then, OvmsServerV2::OvmsServerV2 creates the reader (and sets MyOvmsServerV2Reader to it) if it is zero. Then OvmsServerV2::~OvmsServerV2 deregisters (clears) the reader, but does not set MyOvmsServerV2Reader to 0.
>> The change is to OvmsServerV2::OvmsServerV2 to register the reader (without setting MyOvmsServerV2Reader) if MyOvmsServerV2Reader is 0.
>> This seems to leak one reader every time the OvmsServerV2 is stopped/started, as without storing in MyOvmsServerV2Reader, the old reader will never be cleared upon deregistering. Or am I missing something?
> You miss the MyOvmsServerV2Reader being given to the second registration call to be reused.
> The current implementation of reader registration is allocating one of 32 available bits for a new reader. The bit allocation is permanent, the servers accordingly only allocate their ids once.
> But the reader entry in the list of active notification readers isn't permanent. To be able to re-register for a previously allocated bit number, I introduced the second RegisterReader() method some time ago. That was for the webserver context -- registering a new id on every new websocket connection would run out of ids very soon.
> It became apparent (from a user report on the weekend) that both v2 & v3 servers did not register as readers on a second start, so have the same issue.
> You can test the previous implementation and the fix by watching "notify status".
>> It seems that the better solution is to change OvmsServerV2::~OvmsServerV2 to set MyOvmsServerV2Reader=0 after deregistering (which is the original bug, imho)?
> That would actually leak reader bits due to the permanent allocation.
> It would have been an option to add a bit mask for the use/free status so you don't need to permanently store and reuse the allocated reader id. IIRC I decided against that because the v2&v3 servers had that scheme of permanent storage. Thinking about it now, that would be a more clean, more standard way option for the notify API, maybe we should do that.
>> Regards, Mark.
>>> Begin forwarded message:
>>> From: Michael Balzer <noreply at github.com <mailto:noreply at github.com>>
>>> Subject: [openvehicles/Open-Vehicle-Monitoring-System-3] f84837: Server V2 & V3: fix registration for notifications...
>>> Date: 17 November 2019 at 11:22:46 PM HKT
>>> To: mark at webb-johnson.net <mailto:mark at webb-johnson.net>
>>> Branch: refs/heads/master
>>> Home: https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3 <https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3>
>>> Commit: f84837a2ce40d32769134d0bb5fe6dee09d842b3
>>> https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/commit/f84837a2ce40d32769134d0bb5fe6dee09d842b3 <https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/commit/f84837a2ce40d32769134d0bb5fe6dee09d842b3>
>>> Author: Michael Balzer <balzer at expeedo.de <mailto:balzer at expeedo.de>>
>>> Date: 2019-11-17 (Sun, 17 Nov 2019)
>>> Changed paths:
>>> M vehicle/OVMS.V3/components/ovms_server_v2/src/ovms_server_v2.cpp
>>> M vehicle/OVMS.V3/components/ovms_server_v3/src/ovms_server_v3.cpp
>>> Log Message:
>>> Server V2 & V3: fix registration for notifications on restart
>> OvmsDev mailing list
>> OvmsDev at lists.openvehicles.com <mailto:OvmsDev at lists.openvehicles.com>
>> http://lists.openvehicles.com/mailman/listinfo/ovmsdev <http://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
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the OvmsDev