<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Michael,<div class=""><br class=""></div><div class="">Thanks for the clarification. That makes more sense.</div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">Worth it, or not?</div><div class=""><br class=""></div><div class="">Regards, Mark.</div><div class=""><div><br class=""><blockquote type="cite" class=""><div class="">On 18 Nov 2019, at 5:35 PM, Michael Balzer <<a href="mailto:dexter@expeedo.de" class="">dexter@expeedo.de</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" class="">
<div class="">
Mark,<br class="">
<br class="">
<div class="moz-cite-prefix">Am 18.11.19 um 02:37 schrieb Mark
Webb-Johnson:<br class="">
</div>
<blockquote type="cite" cite="mid:C49BFD54-D155-4A63-8385-A076B62FDB49@webb-johnson.net" class="">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" class="">
I don’t think this fix is correct. Seeking clarification of the
thinking behind it.
<div class=""><br class="">
</div>
<div class="">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.</div>
<div class=""><br class="">
</div>
<div class="">The change is to OvmsServerV2::OvmsServerV2 to
register the reader (without setting MyOvmsServerV2Reader)
if MyOvmsServerV2Reader is 0.</div>
<div class=""><br class="">
</div>
<div class="">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?</div>
</blockquote>
<br class="">
You miss the MyOvmsServerV2Reader being given to the second
registration call to be reused.<br class="">
<br class="">
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.<br class="">
<br class="">
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.<br class="">
<br class="">
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.<br class="">
<br class="">
You can test the previous implementation and the fix by watching
"notify status".<br class="">
<br class="">
<blockquote type="cite" cite="mid:C49BFD54-D155-4A63-8385-A076B62FDB49@webb-johnson.net" class="">
<div class="">It seems that the better solution is to
change OvmsServerV2::~OvmsServerV2 to set MyOvmsServerV2Reader=0
after deregistering (which is the original bug, imho)?</div>
</blockquote>
<br class="">
That would actually leak reader bits due to the permanent
allocation.<br class="">
<br class="">
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.<br class="">
<br class="">
Regards,<br class="">
Michael<br class="">
<br class="">
<br class="">
<blockquote type="cite" cite="mid:C49BFD54-D155-4A63-8385-A076B62FDB49@webb-johnson.net" class="">
<div class="">Regards, Mark.<br class="">
<div class=""><br class="">
<blockquote type="cite" class="">
<div class="">Begin forwarded message:</div>
<br class="Apple-interchange-newline">
<div style="margin-top: 0px; margin-right: 0px;
margin-bottom: 0px; margin-left: 0px;" class=""><span style="font-family: -webkit-system-font, "Helvetica Neue", Helvetica, sans-serif;" class=""><b class="">From: </b></span><span style="font-family: -webkit-system-font, Helvetica Neue,
Helvetica, sans-serif;" class="">Michael Balzer <<a href="mailto:noreply@github.com" class="" moz-do-not-send="true">noreply@github.com</a>><br class="">
</span></div>
<div style="margin-top: 0px; margin-right: 0px;
margin-bottom: 0px; margin-left: 0px;" class=""><span style="font-family: -webkit-system-font, "Helvetica Neue", Helvetica, sans-serif;" class=""><b class="">Subject: </b></span><span style="font-family: -webkit-system-font, Helvetica Neue,
Helvetica, sans-serif;" class=""><b class="">[openvehicles/Open-Vehicle-Monitoring-System-3]
f84837: Server V2 & V3: fix registration for
notifications...</b><br class="">
</span></div>
<div style="margin-top: 0px; margin-right: 0px;
margin-bottom: 0px; margin-left: 0px;" class=""><span style="font-family: -webkit-system-font, "Helvetica Neue", Helvetica, sans-serif;" class=""><b class="">Date: </b></span><span style="font-family: -webkit-system-font, Helvetica Neue,
Helvetica, sans-serif;" class="">17 November 2019 at
11:22:46 PM HKT<br class="">
</span></div>
<div style="margin-top: 0px; margin-right: 0px;
margin-bottom: 0px; margin-left: 0px;" class=""><span style="font-family: -webkit-system-font, "Helvetica Neue", Helvetica, sans-serif;" class=""><b class="">To: </b></span><span style="font-family: -webkit-system-font, Helvetica Neue,
Helvetica, sans-serif;" class=""><a href="mailto:mark@webb-johnson.net" class="" moz-do-not-send="true">mark@webb-johnson.net</a><br class="">
</span></div>
<br class="">
<div class="">
<div class=""> Branch: refs/heads/master<br class="">
Home: <a href="https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3" class="" moz-do-not-send="true">https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3</a><br class="">
Commit: f84837a2ce40d32769134d0bb5fe6dee09d842b3<br class="">
<a href="https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/commit/f84837a2ce40d32769134d0bb5fe6dee09d842b3" class="" moz-do-not-send="true">https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/commit/f84837a2ce40d32769134d0bb5fe6dee09d842b3</a><br class="">
Author: Michael Balzer <<a href="mailto:balzer@expeedo.de" class="" moz-do-not-send="true">balzer@expeedo.de</a>><br class="">
Date: 2019-11-17 (Sun, 17 Nov 2019)<br class="">
<br class="">
Changed paths:<br class="">
M
vehicle/OVMS.V3/components/ovms_server_v2/src/ovms_server_v2.cpp<br class="">
M
vehicle/OVMS.V3/components/ovms_server_v3/src/ovms_server_v3.cpp<br class="">
<br class="">
Log Message:<br class="">
-----------<br class="">
Server V2 & V3: fix registration for notifications
on restart<br class="">
<br class="">
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
<br class="">
<fieldset class="mimeAttachmentHeader"></fieldset>
<pre class="moz-quote-pre" wrap="">_______________________________________________
OvmsDev mailing list
<a class="moz-txt-link-abbreviated" href="mailto:OvmsDev@lists.openvehicles.com">OvmsDev@lists.openvehicles.com</a>
<a class="moz-txt-link-freetext" href="http://lists.openvehicles.com/mailman/listinfo/ovmsdev">http://lists.openvehicles.com/mailman/listinfo/ovmsdev</a>
</pre>
</blockquote>
<br class="">
<pre class="moz-signature" cols="160">--
Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
</pre>
</div>
_______________________________________________<br class="">OvmsDev mailing list<br class=""><a href="mailto:OvmsDev@lists.openvehicles.com" class="">OvmsDev@lists.openvehicles.com</a><br class="">http://lists.openvehicles.com/mailman/listinfo/ovmsdev<br class=""></div></blockquote></div><br class=""></div></body></html>