[Ovmsdev] Fwd: [openvehicles/Open-Vehicle-Monitoring-System-3] f84837: Server V2 & V3: fix registration for notifications...

[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?

It seems that the better solution is to change OvmsServerV2::~OvmsServerV2 to set MyOvmsServerV2Reader=0 after deregistering (which is the original bug, imho)?

Regards, Mark.

> Begin forwarded message:
> 
> From: Michael Balzer <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
> 
>  Branch: refs/heads/master
>  Home:   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>
>  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
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvehicles.com/pipermail/ovmsdev/attachments/20191118/bea759ee/attachment.html>