Skip to content

Commit

Permalink
fix a race condition
Browse files Browse the repository at this point in the history
Service name should not be claimed on dbus before the service
is fully constructed, otherwise other services can end up
scanning a service that does not (yet) have all the mandatory
paths, or support all the dbus calls, causing such services
to be ignored by the GUI and others.

This fixes two issues:

1. GetItems on / is implemented the moment NameOwnerChanged
   is fired
2. /DeviceInstance is available immediately, which many
   consumers actually require.

victronenergy/venus-private#410
  • Loading branch information
izak committed Jul 10, 2024
1 parent 838c12a commit 494f9ae
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 4 deletions.
4 changes: 4 additions & 0 deletions test/mock_dbus_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ def add_path(self, path, value, description="", writeable=False, onchangecallbac
if onchangecallback is not None:
self._callbacks[path] = onchangecallback

def register(self):
# Nothing to do when mocking
pass

# Add the mandatory paths, as per victron dbus api doc
def add_mandatory_paths(self, processname, processversion, connection,
deviceinstance, productid, productname, firmwareversion, hardwareversion, connected):
Expand Down
9 changes: 5 additions & 4 deletions vedbus.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ def __init__(self, servicename, bus=None):
self._dbusnodes = {}
self._ratelimiters = []
self._dbusname = None
self.name = servicename

# dict containing the onchange callbacks, for each object. Object path is the key
self._onchangecallbacks = {}
Expand All @@ -74,13 +75,13 @@ def __init__(self, servicename, bus=None):
# make the dbus connection available to outside, could make this a true property instead, but ach..
self.dbusconn = self._dbusconn

# Register ourselves on the dbus, trigger an error if already in use (do_not_queue)
self._dbusname = dbus.service.BusName(servicename, self._dbusconn, do_not_queue=True)

# Add the root item that will return all items as a tree
self._dbusnodes['/'] = VeDbusRootExport(self._dbusconn, '/', self)

logging.info("registered ourselves on D-Bus as %s" % servicename)
def register(self):
# Register ourselves on the dbus, trigger an error if already in use (do_not_queue)
self._dbusname = dbus.service.BusName(self.name, self._dbusconn, do_not_queue=True)
logging.info("registered ourselves on D-Bus as %s" % self.name)

# To force immediate deregistering of this dbus service and all its object paths, explicitly
# call __del__().
Expand Down

0 comments on commit 494f9ae

Please sign in to comment.