Enhance the DBus interface for peerd

chromeos/dbus/peer_daemon_manager_client.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROMEOS_DBUS_PEER_DAEMON_MANAGER_CLIENT_H_
 #define CHROMEOS_DBUS_PEER_DAEMON_MANAGER_CLIENT_H_
 
 #include <map>
 #include <string>
+#include <tuple>

[hashimoto, 2015/02/05 09:51:12]

<tuple> is not allowed yet. http://chromium-cpp.appspot.com/

[dtapuska, 2015/02/05 19:51:16]

Done.

 #include <vector>
 
 #include "base/macros.h"
 #include "base/values.h"
 #include "chromeos/chromeos_export.h"
 #include "chromeos/dbus/dbus_client.h"
 #include "chromeos/dbus/dbus_method_call_status.h"
+#include "dbus/property.h"
 
 namespace chromeos {
 
 // PeerDaemonManagerClient is used to communicate with the PeerDaemon Manager
 // service.  All methods should be called from the origin thread which
 // initializes the DBusThreadManager instance.
 class CHROMEOS_EXPORT PeerDaemonManagerClient : public DBusClient {
  public:
+  class ManagerProperties : public dbus::PropertySet {
+   public:
+    ManagerProperties(dbus::ObjectProxy* object_proxy,
+                      const PropertyChangedCallback& callback);
+    ~ManagerProperties() override;
+
+    dbus::Property<std::vector<std::string>> monitored_technologies;

[hashimoto, 2015/02/05 09:51:12]

Could you make this and other properties private?

[dtapuska, 2015/02/05 19:51:17]

I'd need to add access methods for these properties. I followed the
bluetooth_device_client.h

Would you prefer I converted this into a struct to exactly follow that code?

[hashimoto, 2015/02/06 07:45:11]

Generally speaking, we shouldn't expose data members public.
(http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Access_Control)
It's always better to explicitly provide setter methods when write-access is
allowed.

If you are sure you want to allow write-access of all properties for random user
code, making this a struct to be consistent with the existing Bluetooth example
might be OK.
(Though, struct consisting of non-POD members look kind of unnatural IMHO.)

[dtapuska, 2015/02/06 15:56:00]

Done.

+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(ManagerProperties);
+  };
+
+  class ServiceProperties : public dbus::PropertySet {
+   public:
+    ServiceProperties(dbus::ObjectProxy* object_proxy,
+                      const PropertyChangedCallback& callback);
+    ~ServiceProperties() override;
+
+    dbus::Property<std::string> service_id;
+    dbus::Property<std::map<std::string, std::string>> service_info;
+    dbus::Property<std::vector<std::tuple<std::vector<uint8_t>, uint16_t>>>
+        ip_infos;
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(ServiceProperties);
+  };
+
+  class PeerProperties : public dbus::PropertySet {
+   public:
+    PeerProperties(dbus::ObjectProxy* object_proxy,
+                   const PropertyChangedCallback& callback);
+    ~PeerProperties() override;
+
+    dbus::Property<std::string> u_u_i_d;

[hashimoto, 2015/02/05 09:51:12]

nit: Seems |uuid| is more common.

[dtapuska, 2015/02/05 19:51:16]

Done.

+    dbus::Property<uint64_t> last_seen;
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(PeerProperties);
+  };
+
+  // Interface for observing changes from a leadership daemon.
+  class Observer {
+   public:
+    virtual ~Observer() {}
+
+    // Called when the peer daemon manager is added.
+    virtual void ManagerAdded() {}
+
+    // Called when the peer daemon manager is removed; perhaps on a process
+    // crash of the peer daemon.
+    virtual void ManagerRemoved() {}
+
+    // Called when the manager changes a property value.
+    virtual void ManagerPropertyChanged(const std::string& property_name) {}
+
+    // Called when the service with object path |object_path| is added to the
+    // system.
+    virtual void ServiceAdded(const dbus::ObjectPath& object_path) {}
+
+    // Called when the service with object path |object_path| is removed from
+    // the system.
+    virtual void ServiceRemoved(const dbus::ObjectPath& object_path) {}
+
+    // Called when the service with object path |object_path| changes a
+    // property value.
+    virtual void ServicePropertyChanged(const dbus::ObjectPath& object_path,
+                                        const std::string& property_name) {}
+
+    // Called when the peer with object path |object_path| is added to the
+    // system.
+    virtual void PeerAdded(const dbus::ObjectPath& object_path) {}
+
+    // Called when the peer with object path |object_path| is removed from
+    // the system.
+    virtual void PeerRemoved(const dbus::ObjectPath& object_path) {}
+
+    // Called when the peer with object path |object_path| changes a
+    // property value.
+    virtual void PeerPropertyChanged(const dbus::ObjectPath& object_path,
+                                     const std::string& property_name) {}
+  };
+
   ~PeerDaemonManagerClient() override;
 
   // Factory function, creates a new instance which is owned by the caller.
   // For normal usage, access the singleton via DBusThreadManager::Get().
   static PeerDaemonManagerClient* Create();
 
+  // Adds and removes observers for events on all peer events.
+  virtual void AddObserver(Observer* observer) = 0;
+  virtual void RemoveObserver(Observer* observer) = 0;
+
+  // Retries a list of all the peers.

[hashimoto, 2015/02/05 09:51:12]

nit: Retrieves?

[dtapuska, 2015/02/05 19:51:16]

Done.

+  virtual std::vector<dbus::ObjectPath> GetPeers() = 0;
+
+  // Retries a list of all the services.

[hashimoto, 2015/02/05 09:51:12]

ditto.

[dtapuska, 2015/02/05 19:51:17]

Done.

+  virtual std::vector<dbus::ObjectPath> GetServices() = 0;
+
+  // Obtain the properties for the peer with object path |object_path|,

[hashimoto, 2015/02/05 09:51:12]

nit: Obtains

[dtapuska, 2015/02/05 19:51:17]

Done.

+  // any values should be copied if needed.
+  virtual PeerProperties* GetPeerProperties(
+      const dbus::ObjectPath& object_path) = 0;
+
+  // Obtain the properties for the service with object path |object_path|,

[hashimoto, 2015/02/05 09:51:12]

ditto.

[dtapuska, 2015/02/05 19:51:16]

Done.

+  // any values should be copied if needed.
+  virtual ServiceProperties* GetServiceProperties(

[hashimoto, 2015/02/05 09:51:12]

nit: Properties and Observer methods are ordered like Manager->Service->Peer,
but in this part it's Peer->Service.
It should be better to be consistent everywhere.

[dtapuska, 2015/02/05 19:51:17]

Done.

+      const dbus::ObjectPath& object_path) = 0;
+
   // Calls StartMonitoring method.
   // |callback| is called with its |call_status| argument set to
   // DBUS_METHOD_CALL_SUCCESS if the method call succeeds. Otherwise,
   // |callback| is called with |call_status| set to DBUS_METHOD_CALL_FAILURE.
   virtual void StartMonitoring(
       const std::vector<std::string>& requested_technologies,
       const base::DictionaryValue& options,
       const StringDBusMethodCallback& callback) = 0;
 
   // Calls StopMonitoring method.
@@ skipping 30 matching lines @@
   // Create() should be used instead.
   PeerDaemonManagerClient();
 
  private:
   DISALLOW_COPY_AND_ASSIGN(PeerDaemonManagerClient);
 };
 
 }  // namespace chromeos
 
 #endif  // CHROMEOS_DBUS_PEER_DAEMON_MANAGER_CLIENT_H_