From e11439d16afbc6c763458f5d7d8d3150952bc7b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mihai=20Bi=C5=9Fog?= Date: Fri, 6 Nov 2020 10:36:03 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9BAdded=20active=20foreign=20device?= =?UTF-8?q?=20registration=20tracking,=20improved=20registration=20behavio?= =?UTF-8?q?r?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The way foreign device registration was handled was a bit confusing. You would call `register` and if everything was perfect then subsequent renewals would be done automatically. Except when the BBMD would sometimes return an error in which case subsequent renewals would stop. - With this change, renewals will always be rescheduled independently of the BBMD's response. You call `register` to start the registration (and renewal) process and `unregister` to bring it to a halt. - Because there was no foreign device registration timeout tracking, and because of the previous issue, BIPForeign could end up in an inconsistent state if it had an active registration and for the next renewal the BBMD would not answer (e.g. poor network conditions). In this case the registration status would remain as registered and no further renewals would have been attempted. - This is solved by implementing timeout tracking of the active registration. The BACnet standard mentions that a BBMD should remove the registration from its FDT if no renewal was done 30s after the TTL, so if (30 + TTL) seconds pass since the last ACK then we've definitely lost the registration. --- py25/bacpypes/bvllservice.py | 57 +++++++++++++++++++++++++++++++----- py27/bacpypes/bvllservice.py | 57 +++++++++++++++++++++++++++++++----- py34/bacpypes/bvllservice.py | 57 +++++++++++++++++++++++++++++++----- 3 files changed, 147 insertions(+), 24 deletions(-) diff --git a/py25/bacpypes/bvllservice.py b/py25/bacpypes/bvllservice.py index b17301d..f047f66 100755 --- a/py25/bacpypes/bvllservice.py +++ b/py25/bacpypes/bvllservice.py @@ -11,7 +11,7 @@ from time import time as _time from .debugging import ModuleLogger, DebugContents, bacpypes_debugging from .udp import UDPDirector -from .task import OneShotTask, RecurringTask +from .task import OneShotFunction, OneShotTask, RecurringTask from .comm import Client, Server, bind, \ ServiceAccessPoint, ApplicationServiceElement @@ -483,6 +483,9 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): self.bbmdAddress = None self.bbmdTimeToLive = None + # used in tracking active registration timeouts + self._registration_timeout_task = OneShotFunction(self._registration_expired) + # registration provided if addr: # a little error checking @@ -539,10 +542,9 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): # save the result code as the status self.registrationStatus = pdu.bvlciResultCode - # check for success - if pdu.bvlciResultCode == 0: - # schedule for a refresh - self.install_task(delta=self.bbmdTimeToLive) + # If successful, track registration timeout + if self.registrationStatus == 0: + self._start_track_registration() return @@ -633,7 +635,11 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): BIPForeign._warning("invalid pdu type: %s", type(pdu)) def register(self, addr, ttl): - """Initiate the process of registering with a BBMD.""" + """Start the foreign device registration process with the given BBMD. + + Registration will be renewed periodically according to the ttl value + until explicitly stopped by a call to `unregister`. + """ # a little error checking if ttl <= 0: raise ValueError("time-to-live must be greater than zero") @@ -645,11 +651,18 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): self.bbmdAddress = Address(addr) self.bbmdTimeToLive = ttl - # install this task to run when it gets a chance + # install this task to do registration renewal according to the TTL + # and stop tracking any active registration timeouts self.install_task(when=0) + self._stop_track_registration() def unregister(self): - """Drop the registration with a BBMD.""" + """Stop the foreign device registration process. + + Immediately drops active foreign device registration and stops further + registration renewals. + """ + pdu = RegisterForeignDevice(0) pdu.pduDestination = self.bbmdAddress @@ -663,6 +676,11 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): self.bbmdAddress = None self.bbmdTimeToLive = None + # unschedule registration renewal & timeout tracking if previously + # scheduled + self.suspend_task() + self._stop_track_registration() + def process_task(self): """Called when the registration request should be sent to the BBMD.""" pdu = RegisterForeignDevice(self.bbmdTimeToLive) @@ -671,6 +689,29 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): # send it downstream self.request(pdu) + # schedule the next registration renewal + self.install_task(delta=self.bbmdTimeToLive) + + def _start_track_registration(self): + # From J.5.2.3 Foreign Device Table Operation (paraphrasing): if a + # foreign device does not renew its registration 30 seconds after its + # TTL expired then it will be removed from the BBMD's FDT. + # + # Thus, if we're registered and don't get a response to a subsequent + # renewal request 30 seconds after our TTL expired then we're + # definitely not registered anymore. + self._registration_timeout_task.install_task(delta=self.bbmdTimeToLive + 30) + + def _stop_track_registration(self): + self._registration_timeout_task.suspend_task() + + def _registration_expired(self): + """Called when detecting that foreign device registration has + definitely expired. + """ + self.registrationStatus = -2 # Unregistered + self._stop_track_registration() + bacpypes_debugging(BIPForeign) # diff --git a/py27/bacpypes/bvllservice.py b/py27/bacpypes/bvllservice.py index 99f9998..69282cc 100755 --- a/py27/bacpypes/bvllservice.py +++ b/py27/bacpypes/bvllservice.py @@ -11,7 +11,7 @@ from .settings import settings from .debugging import ModuleLogger, DebugContents, bacpypes_debugging from .udp import UDPDirector -from .task import OneShotTask, RecurringTask +from .task import OneShotFunction, OneShotTask, RecurringTask from .comm import Client, Server, bind, \ ServiceAccessPoint, ApplicationServiceElement @@ -481,6 +481,9 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): self.bbmdAddress = None self.bbmdTimeToLive = None + # used in tracking active registration timeouts + self._registration_timeout_task = OneShotFunction(self._registration_expired) + # registration provided if addr: # a little error checking @@ -537,10 +540,9 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): # save the result code as the status self.registrationStatus = pdu.bvlciResultCode - # check for success - if pdu.bvlciResultCode == 0: - # schedule for a refresh - self.install_task(delta=self.bbmdTimeToLive) + # If successful, track registration timeout + if self.registrationStatus == 0: + self._start_track_registration() return @@ -631,7 +633,11 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): BIPForeign._warning("invalid pdu type: %s", type(pdu)) def register(self, addr, ttl): - """Initiate the process of registering with a BBMD.""" + """Start the foreign device registration process with the given BBMD. + + Registration will be renewed periodically according to the ttl value + until explicitly stopped by a call to `unregister`. + """ # a little error checking if ttl <= 0: raise ValueError("time-to-live must be greater than zero") @@ -643,11 +649,18 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): self.bbmdAddress = Address(addr) self.bbmdTimeToLive = ttl - # install this task to run when it gets a chance + # install this task to do registration renewal according to the TTL + # and stop tracking any active registration timeouts self.install_task(when=0) + self._stop_track_registration() def unregister(self): - """Drop the registration with a BBMD.""" + """Stop the foreign device registration process. + + Immediately drops active foreign device registration and stops further + registration renewals. + """ + pdu = RegisterForeignDevice(0) pdu.pduDestination = self.bbmdAddress @@ -661,6 +674,11 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): self.bbmdAddress = None self.bbmdTimeToLive = None + # unschedule registration renewal & timeout tracking if previously + # scheduled + self.suspend_task() + self._stop_track_registration() + def process_task(self): """Called when the registration request should be sent to the BBMD.""" pdu = RegisterForeignDevice(self.bbmdTimeToLive) @@ -669,6 +687,29 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): # send it downstream self.request(pdu) + # schedule the next registration renewal + self.install_task(delta=self.bbmdTimeToLive) + + def _start_track_registration(self): + # From J.5.2.3 Foreign Device Table Operation (paraphrasing): if a + # foreign device does not renew its registration 30 seconds after its + # TTL expired then it will be removed from the BBMD's FDT. + # + # Thus, if we're registered and don't get a response to a subsequent + # renewal request 30 seconds after our TTL expired then we're + # definitely not registered anymore. + self._registration_timeout_task.install_task(delta=self.bbmdTimeToLive + 30) + + def _stop_track_registration(self): + self._registration_timeout_task.suspend_task() + + def _registration_expired(self): + """Called when detecting that foreign device registration has + definitely expired. + """ + self.registrationStatus = -2 # Unregistered + self._stop_track_registration() + # # BIPBBMD # diff --git a/py34/bacpypes/bvllservice.py b/py34/bacpypes/bvllservice.py index 5c334f9..ef0894f 100755 --- a/py34/bacpypes/bvllservice.py +++ b/py34/bacpypes/bvllservice.py @@ -10,7 +10,7 @@ from .settings import settings from .debugging import ModuleLogger, DebugContents, bacpypes_debugging from .udp import UDPDirector -from .task import OneShotTask, RecurringTask +from .task import OneShotFunction, OneShotTask, RecurringTask from .comm import Client, Server, bind, \ ServiceAccessPoint, ApplicationServiceElement @@ -495,6 +495,9 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): self.bbmdAddress = None self.bbmdTimeToLive = None + # used in tracking active registration timeouts + self._registration_timeout_task = OneShotFunction(self._registration_expired) + # registration provided if addr: # a little error checking @@ -555,10 +558,9 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): # save the result code as the status self.registrationStatus = pdu.bvlciResultCode - # check for success - if pdu.bvlciResultCode == 0: - # schedule for a refresh - self.install_task(delta=self.bbmdTimeToLive) + # If successful, track registration timeout + if self.registrationStatus == 0: + self._start_track_registration() return @@ -651,7 +653,11 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): BIPForeign._warning("invalid pdu type: %s", type(pdu)) def register(self, addr, ttl): - """Initiate the process of registering with a BBMD.""" + """Start the foreign device registration process with the given BBMD. + + Registration will be renewed periodically according to the ttl value + until explicitly stopped by a call to `unregister`. + """ # a little error checking if ttl <= 0: raise ValueError("time-to-live must be greater than zero") @@ -663,11 +669,18 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): self.bbmdAddress = Address(addr) self.bbmdTimeToLive = ttl - # install this task to run when it gets a chance + # install this task to do registration renewal according to the TTL + # and stop tracking any active registration timeouts self.install_task(when=0) + self._stop_track_registration() def unregister(self): - """Drop the registration with a BBMD.""" + """Stop the foreign device registration process. + + Immediately drops active foreign device registration and stops further + registration renewals. + """ + pdu = RegisterForeignDevice(0) pdu.pduDestination = self.bbmdAddress @@ -681,6 +694,11 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): self.bbmdAddress = None self.bbmdTimeToLive = None + # unschedule registration renewal & timeout tracking if previously + # scheduled + self.suspend_task() + self._stop_track_registration() + def process_task(self): """Called when the registration request should be sent to the BBMD.""" pdu = RegisterForeignDevice(self.bbmdTimeToLive) @@ -689,6 +707,29 @@ class BIPForeign(BIPSAP, Client, Server, OneShotTask, DebugContents): # send it downstream self.request(pdu) + # schedule the next registration renewal + self.install_task(delta=self.bbmdTimeToLive) + + def _start_track_registration(self): + # From J.5.2.3 Foreign Device Table Operation (paraphrasing): if a + # foreign device does not renew its registration 30 seconds after its + # TTL expired then it will be removed from the BBMD's FDT. + # + # Thus, if we're registered and don't get a response to a subsequent + # renewal request 30 seconds after our TTL expired then we're + # definitely not registered anymore. + self._registration_timeout_task.install_task(delta=self.bbmdTimeToLive + 30) + + def _stop_track_registration(self): + self._registration_timeout_task.suspend_task() + + def _registration_expired(self): + """Called when detecting that foreign device registration has + definitely expired. + """ + self.registrationStatus = -2 # Unregistered + self._stop_track_registration() + # # BIPBBMD #