Hello Pau Espin Pedrol, Jenkins Builder, I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/2554 to look at the new patch set (#5). ofono/dbus: detach from signals as appropriate In the dbus_connect() code path, return the subscription token. In the Modem class, use this token to disconnect signal callbacks when an interface is removed. Generalize the signal connect/disconnect handling: have one tuple defining all signals and their handler functions, add generalized loop to attach them. Store all subscription tokens in a dict of lists, any number of signal callbacks per general interface name. When an interface is announced to be removed from ofono, detach all signals for that interface implicitly. So far this only handles the MessageManager interface's IncomingMessage signal, but others will likely follow soon. Fixes: OS#2242 Change-Id: I0939ef414bc599ee8742df48da04d8d9569d00ba --- M src/osmo_gsm_tester/ofono_client.py 1 file changed, 28 insertions(+), 16 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-gsm-tester refs/changes/54/2554/5 diff --git a/src/osmo_gsm_tester/ofono_client.py b/src/osmo_gsm_tester/ofono_client.py index 5080628..faa9192 100644 --- a/src/osmo_gsm_tester/ofono_client.py +++ b/src/osmo_gsm_tester/ofono_client.py @@ -17,7 +17,7 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. -from . import log, test +from . import log, test, util from pydbus import SystemBus, Variant import time @@ -37,7 +37,7 @@ def __init__(self, dbus_iface, handler): self.handler = handler - dbus_iface.connect(self.receive_signal) + self.subscription_id = dbus_iface.connect(self.receive_signal) def receive_signal(self, *args, **kwargs): DeferredHandling.defer_queue.append((self.handler, args, kwargs)) @@ -55,7 +55,7 @@ so that a signal handler is invoked only after the DBus polling is through by enlisting signals that should be handled in the DeferredHandling.defer_queue.''' - DeferredHandling(dbus_iface, handler) + return DeferredHandling(dbus_iface, handler).subscription_id def poll(): @@ -85,6 +85,7 @@ self.set_log_category(log.C_BUS) self._dbus_obj = None self._interfaces = set() + self._connected_signals = util.listdict() self.sms_received_list = [] # init interfaces and connect to signals: self.dbus_obj() @@ -149,24 +150,35 @@ for iface in additions: self._on_interface_enabled(iface) + def _disconnect(self, interface_name): + subscriptions = self._connected_signals.pop(interface_name, []) + if subscriptions: + self.dbg('Disconnecting', len(subscriptions), 'signals from', interface_name) + for subscription in subscriptions: + subscription.disconnect() + def _on_interface_enabled(self, interface_name): self.dbg('Interface enabled:', interface_name) - if interface_name == I_SMS: - retries = 3 - while True: - try: - dbus_connect(self.dbus_obj()[I_SMS].IncomingMessage, self._on_incoming_message) - break - except KeyError: - self.dbg('Interface not yet available:', I_SMS) - retries -= 1 - time.sleep(1) - if retries <= 0: - self.err('Interface enabled by signal, but not available:', I_SMS) - raise + + all_wanted_conns = { + I_SMS: ( ('IncomingMessage', self._on_incoming_message), ), + } + + want = all_wanted_conns.get(interface_name) + if not want: + return + + # sanity + self._disconnect(interface_name) + + self.dbg('Connecting', len(want), 'signals to', interface_name) + for signal, cb in want: + dbus_iface = getattr(self.dbus_obj()[interface_name], signal) + self._connected_signals.add(interface_name, dbus_connect(dbus_iface, cb)) def _on_interface_disabled(self, interface_name): self.dbg('Interface disabled:', interface_name) + self._disconnect(interface_name) def has_interface(self, name): return name in self._interfaces -- To view, visit https://gerrit.osmocom.org/2554 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0939ef414bc599ee8742df48da04d8d9569d00ba Gerrit-PatchSet: 5 Gerrit-Project: osmo-gsm-tester Gerrit-Branch: master Gerrit-Owner: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de>