refactor: ble interfaces (#715)

* services/ble/GattClient: Require less information for characteristic and descriptor discovery

* Update TestGattClient

* Gatt.proto: Remove Stop*Discovery

* services/ble/Gatt: Add GattDescriptor fields and accessors

* services/ble/GattClient: Remove const from interface methods

* services/ble/GattClient: Fix order of methods

* Improve tracing of not found service methods

* services/ble/Gatt: Add comparison operator to GattDescriptor

* Require CXX standard 20 for use of spaceship operator

* Fix infra/util/Endian for C++20

* TestCucumberWireProtocolServer: Resolve warning

* infra/timer/Timer.hpp: Remove PrintTo from namespace std

* Revert "infra/timer/Timer.hpp: Remove PrintTo from namespace std"

This reverts commit 6048fe6.

* infra/util/CMakeLists: set permissive for VS2019

* infra/util/CMakeLists: set permissive for VS2019

* Undo usage of C++20

* infra/util/Variant: Add default move constructor/assignment operator with noexcept specification

infra/event/EventDispatcher.cpp

@@ -65,6 +65,10 @@ namespace infra
         {
             struct ExceptionSafePop
             {
+                ExceptionSafePop(EventDispatcherWorkerImpl& worker)
+                    : worker(worker)
+                {}
+
                 ExceptionSafePop(const ExceptionSafePop&) = delete;
                 ExceptionSafePop& operator=(const ExceptionSafePop&) = delete;
 

infra/event/EventDispatcherWithWeakPtr.cpp

@@ -81,6 +81,10 @@ namespace infra
         {
             struct ExceptionSafePop
             {
+                ExceptionSafePop(EventDispatcherWithWeakPtrWorker& worker)
+                    : worker(worker)
+                {}
+
                 ExceptionSafePop(const ExceptionSafePop&) = delete;
                 ExceptionSafePop& operator=(const ExceptionSafePop&) = delete;
 

infra/util/Endian.hpp

@@ -163,24 +163,24 @@ namespace infra
 
         friend bool operator==(T x, BigEndian y)
         {
-            return y == x;
+            return y.operator==(x);
         }
 
         friend bool operator!=(T x, BigEndian y)
         {
-            return y != x;
+            return y.operator!=(x);
         }
 
         template<class U>
         friend bool operator==(U x, BigEndian y)
         {
-            return y == x;
+            return y.operator==(x);
         }
 
         template<class U>
         friend bool operator!=(U x, BigEndian y)
         {
-            return y != x;
+            return y.operator!=(x);
         }
 
     private:

infra/util/Variant.hpp

@@ -20,6 +20,7 @@ namespace infra
 
         Variant();
         Variant(const Variant& other);
+        Variant(Variant&& other) noexcept((std::is_nothrow_move_constructible_v<T> && ...)) = default;
         template<class... T2>
         Variant(const Variant<T2...>& other);
         template<class U>
@@ -30,6 +31,7 @@ namespace infra
         Variant(AtIndex, std::size_t index, Args&&... args);
 
         Variant& operator=(const Variant& other);
+        Variant& operator=(Variant&& other) noexcept((std::is_nothrow_move_assignable_v<T> && ...) && (std::is_nothrow_move_constructible_v<T> && ...)) = default;
         template<class... T2>
         Variant& operator=(const Variant<T2...>& other);
         template<class U>

protobuf/echo/Serialization.cpp

@@ -11,8 +11,6 @@ namespace services
     {
         while (!reader->Empty())
             reader->ExtractContiguousRange(std::numeric_limits<uint32_t>::max());
-
-        reader = nullptr;
     }
 
     void MethodDeserializerDummy::ExecuteMethod()

protobuf/echo/TracingEcho.cpp

@@ -173,7 +173,7 @@ namespace services
 
                 if (contents.Is<infra::PartialProtoLengthDelimited>() && contents.Get<infra::PartialProtoLengthDelimited>().length > stream.Available())
                 {
-                    tracer.Trace() << "< message too big";
+                    tracer.Trace() << "< message too big for service " << serviceId << " method " << methodId << " length " << contents.Get<infra::PartialProtoLengthDelimited>().length << " available " << stream.Available();
                     skipping = contents.Get<infra::PartialProtoLengthDelimited>().length;
                     writerBuffer.erase(writerBuffer.begin(), writerBuffer.begin() + stream.Reader().Processed());
                     auto skippingNow = std::min<std::size_t>(skipping, writerBuffer.size());

protobuf/protoc_echo_plugin/ProtoCEchoPlugin.cpp

@@ -1584,7 +1584,7 @@ switch (methodId)
                 printer.Print("}\n");
             }
             else
-                printer.Print(R"(tracer.Continue() << "$servicename$ method " << methodId << " not found";\n)", "servicename", service->name);
+                printer.Print(R"(tracer.Continue() << "$servicename$ method " << methodId << " not found";\ncontents.SkipEverything();\n)", "servicename", service->name);
         }
 
         return result.str();

services/ble/Gatt.cpp

@@ -2,6 +2,36 @@
 
 namespace services
 {
+    GattDescriptor::GattDescriptor(const AttAttribute::Uuid& type, AttAttribute::Handle handle)
+        : type(type)
+        , handle(handle)
+    {}
+
+    const AttAttribute::Uuid& GattDescriptor::Type() const
+    {
+        return type;
+    }
+
+    AttAttribute::Handle GattDescriptor::Handle() const
+    {
+        return handle;
+    }
+
+    AttAttribute::Handle& GattDescriptor::Handle()
+    {
+        return handle;
+    }
+
+    bool GattDescriptor::operator==(const GattDescriptor& other) const
+    {
+        return type == other.type && handle == other.handle;
+    }
+
+    bool GattDescriptor::operator!=(const GattDescriptor& other) const
+    {
+        return !(*this == other);
+    }
+
     GattCharacteristic::GattCharacteristic(const AttAttribute::Uuid& type, AttAttribute::Handle handle, AttAttribute::Handle valueHandle, GattCharacteristic::PropertyFlags properties)
         : type(type)
         , handle(handle)

services/ble/Gatt.hpp

@@ -35,6 +35,21 @@ namespace services
                 enableIndication = 0x0002,
             };
         };
+
+        GattDescriptor(const AttAttribute::Uuid& type, AttAttribute::Handle handle);
+        GattDescriptor() = default;
+
+        const AttAttribute::Uuid& Type() const;
+
+        AttAttribute::Handle Handle() const;
+        AttAttribute::Handle& Handle();
+
+        bool operator==(const GattDescriptor& other) const;
+        bool operator!=(const GattDescriptor& other) const;
+
+    private:
+        AttAttribute::Uuid type;
+        AttAttribute::Handle handle;
     };
 
     namespace uuid
@@ -72,9 +87,6 @@ namespace services
     public:
         GattCharacteristic(const AttAttribute::Uuid& type, AttAttribute::Handle handle, AttAttribute::Handle valueHandle, PropertyFlags properties);
         GattCharacteristic() = default;
-        GattCharacteristic(GattCharacteristic& other) = delete;
-        GattCharacteristic& operator=(const GattCharacteristic& other) = delete;
-        virtual ~GattCharacteristic() = default;
 
         const PropertyFlags& Properties() const;
         const AttAttribute::Uuid& Type() const;
@@ -96,9 +108,6 @@ namespace services
     public:
         GattService(const AttAttribute::Uuid& type);
         GattService(const AttAttribute::Uuid& type, AttAttribute::Handle handle, AttAttribute::Handle endHandle);
-        GattService(GattService& other) = delete;
-        GattService& operator=(const GattService& other) = delete;
-        virtual ~GattService() = default;
 
         AttAttribute::Uuid Type() const;
         AttAttribute::Handle Handle() const;

services/ble/Gatt.proto

@@ -94,13 +94,8 @@ service GattClient
     rpc MtuSizeExchangeRequest(Nothing) returns (Nothing) { option (method_id) = 1; }
 
     rpc StartServiceDiscovery(Nothing) returns (Nothing) { option (method_id) = 2; }
-    rpc StopServiceDiscovery(Nothing) returns (Nothing) { option (method_id) = 3; }
-
     rpc StartCharacteristicDiscovery(HandleRange) returns (Nothing) { option (method_id) = 4; }
-    rpc StopCharacteristicDiscovery(Nothing) returns (Nothing) { option (method_id) = 5; }
-
     rpc StartDescriptorDiscovery(HandleRange) returns (Nothing) { option (method_id) = 6; }
-    rpc StopDescriptorDiscovery(Nothing) returns (Nothing) { option (method_id) = 7; }
 
     rpc Read(Handle) returns (Nothing) { option (method_id) = 8; }
     rpc Write(CharacteristicData) returns (Nothing) { option (method_id) = 9; }

services/ble/GattClient.cpp

@@ -148,13 +148,13 @@ namespace services
         GattClientDiscoveryObserver::Subject().StartServiceDiscovery();
     }
 
-    void GattClientDiscoveryDecorator::StartCharacteristicDiscovery(const GattService& service)
+    void GattClientDiscoveryDecorator::StartCharacteristicDiscovery(AttAttribute::Handle handle, AttAttribute::Handle endHandle)
     {
-        GattClientDiscoveryObserver::Subject().StartCharacteristicDiscovery(service);
+        GattClientDiscoveryObserver::Subject().StartCharacteristicDiscovery(handle, endHandle);
     }
 
-    void GattClientDiscoveryDecorator::StartDescriptorDiscovery(const GattService& service)
+    void GattClientDiscoveryDecorator::StartDescriptorDiscovery(AttAttribute::Handle handle, AttAttribute::Handle endHandle)
     {
-        GattClientDiscoveryObserver::Subject().StartDescriptorDiscovery(service);
+        GattClientDiscoveryObserver::Subject().StartDescriptorDiscovery(handle, endHandle);
     }
 }

services/ble/GattClient.hpp

@@ -50,14 +50,14 @@ namespace services
         , public infra::Subject<GattClientStackUpdateObserver>
     {
     public:
-        virtual void Read(const GattClientCharacteristicOperationsObserver& characteristic, const infra::Function<void(const infra::ConstByteRange&)>& onRead) const = 0;
-        virtual void Write(const GattClientCharacteristicOperationsObserver& characteristic, infra::ConstByteRange data, const infra::Function<void()>& onDone) const = 0;
-        virtual void WriteWithoutResponse(const GattClientCharacteristicOperationsObserver& characteristic, infra::ConstByteRange data) const = 0;
-
-        virtual void EnableNotification(const GattClientCharacteristicOperationsObserver& characteristic, const infra::Function<void()>& onDone) const = 0;
-        virtual void DisableNotification(const GattClientCharacteristicOperationsObserver& characteristic, const infra::Function<void()>& onDone) const = 0;
-        virtual void EnableIndication(const GattClientCharacteristicOperationsObserver& characteristic, const infra::Function<void()>& onDone) const = 0;
-        virtual void DisableIndication(const GattClientCharacteristicOperationsObserver& characteristic, const infra::Function<void()>& onDone) const = 0;
+        virtual void Read(const GattClientCharacteristicOperationsObserver& characteristic, const infra::Function<void(const infra::ConstByteRange&)>& onRead) = 0;
+        virtual void Write(const GattClientCharacteristicOperationsObserver& characteristic, infra::ConstByteRange data, const infra::Function<void()>& onDone) = 0;
+        virtual void WriteWithoutResponse(const GattClientCharacteristicOperationsObserver& characteristic, infra::ConstByteRange data) = 0;
+
+        virtual void EnableNotification(const GattClientCharacteristicOperationsObserver& characteristic, const infra::Function<void()>& onDone) = 0;
+        virtual void DisableNotification(const GattClientCharacteristicOperationsObserver& characteristic, const infra::Function<void()>& onDone) = 0;
+        virtual void EnableIndication(const GattClientCharacteristicOperationsObserver& characteristic, const infra::Function<void()>& onDone) = 0;
+        virtual void DisableIndication(const GattClientCharacteristicOperationsObserver& characteristic, const infra::Function<void()>& onDone) = 0;
     };
 
     class GattClientCharacteristic
@@ -111,11 +111,10 @@ namespace services
         using infra::Observer<GattClientDiscoveryObserver, GattClientDiscovery>::Observer;
 
         virtual void ServiceDiscovered(const AttAttribute::Uuid& type, AttAttribute::Handle handle, AttAttribute::Handle endHandle) = 0;
-        virtual void CharacteristicDiscovered(const AttAttribute::Uuid& type, AttAttribute::Handle handle, AttAttribute::Handle valueHandle, GattCharacteristic::PropertyFlags properties) = 0;
-        virtual void DescriptorDiscovered(const AttAttribute::Uuid& type, AttAttribute::Handle handle) = 0;
-
         virtual void ServiceDiscoveryComplete() = 0;
+        virtual void CharacteristicDiscovered(const AttAttribute::Uuid& type, AttAttribute::Handle handle, AttAttribute::Handle valueHandle, GattCharacteristic::PropertyFlags properties) = 0;
         virtual void CharacteristicDiscoveryComplete() = 0;
+        virtual void DescriptorDiscovered(const AttAttribute::Uuid& type, AttAttribute::Handle handle) = 0;
         virtual void DescriptorDiscoveryComplete() = 0;
     };
 
@@ -124,8 +123,18 @@ namespace services
     {
     public:
         virtual void StartServiceDiscovery() = 0;
-        virtual void StartCharacteristicDiscovery(const GattService& service) = 0;
-        virtual void StartDescriptorDiscovery(const GattService& service) = 0;
+        virtual void StartCharacteristicDiscovery(AttAttribute::Handle handle, AttAttribute::Handle endHandle) = 0;
+        virtual void StartDescriptorDiscovery(AttAttribute::Handle handle, AttAttribute::Handle endHandle) = 0;
+
+        void StartCharacteristicDiscovery(const GattService& service)
+        {
+            StartCharacteristicDiscovery(service.Handle(), service.EndHandle());
+        }
+
+        void StartDescriptorDiscovery(const GattService& service)
+        {
+            StartDescriptorDiscovery(service.Handle(), service.EndHandle());
+        }
     };
 
     class GattClientDiscoveryDecorator
@@ -146,8 +155,8 @@ namespace services
 
         // Implementation of GattClientDiscovery
         void StartServiceDiscovery() override;
-        void StartCharacteristicDiscovery(const GattService& service) override;
-        void StartDescriptorDiscovery(const GattService& service) override;
+        void StartCharacteristicDiscovery(AttAttribute::Handle handle, AttAttribute::Handle endHandle) override;
+        void StartDescriptorDiscovery(AttAttribute::Handle handle, AttAttribute::Handle endHandle) override;
     };
 }
 

services/ble/GattServerCharacteristicImpl.hpp

@@ -13,7 +13,7 @@ namespace services
         GattServerCharacteristicImpl(GattServerService& service, const AttAttribute::Uuid& type, uint16_t valueLength);
         GattServerCharacteristicImpl(GattServerService& service, const AttAttribute::Uuid& type, uint16_t valueLength, PropertyFlags properties);
         GattServerCharacteristicImpl(GattServerService& service, const AttAttribute::Uuid& type, uint16_t valueLength, PropertyFlags properties, PermissionFlags permissions);
-        ~GattServerCharacteristicImpl() override;
+        ~GattServerCharacteristicImpl();
 
         // Implementation of GattServerCharacteristic
         void Update(infra::ConstByteRange data, infra::Function<void()> onDone) override;

services/ble/test/TestGattClient.cpp

@@ -243,14 +243,12 @@ TEST_F(GattClientDiscoveryDecoratorTest, forward_descriptors_discovered_event_to
 
 TEST_F(GattClientDiscoveryDecoratorTest, forward_all_calls_to_subject)
 {
-    services::GattService service{ uuid16, 0x1, 0x9 };
-
     EXPECT_CALL(gattDiscovery, StartServiceDiscovery());
     decorator.StartServiceDiscovery();
 
-    EXPECT_CALL(gattDiscovery, StartCharacteristicDiscovery(::testing::Ref(service)));
-    decorator.StartCharacteristicDiscovery(service);
+    EXPECT_CALL(gattDiscovery, StartCharacteristicDiscovery(1, 9));
+    decorator.StartCharacteristicDiscovery(1, 9);
 
-    EXPECT_CALL(gattDiscovery, StartDescriptorDiscovery(::testing::Ref(service)));
-    decorator.StartDescriptorDiscovery(service);
+    EXPECT_CALL(gattDiscovery, StartDescriptorDiscovery(1, 9));
+    decorator.StartDescriptorDiscovery(1, 9);
 }

services/ble/test_doubles/GattClientMock.hpp

@@ -14,18 +14,26 @@ namespace services
         MOCK_METHOD(void, UpdateReceived, (infra::ConstByteRange data), (override));
     };
 
+    class GattClientCharacteristicOperationsObserverMock
+        : public services::GattClientCharacteristicOperationsObserver
+    {
+    public:
+        MOCK_METHOD(AttAttribute::Handle, CharacteristicValueHandle, (), (const, override));
+        MOCK_METHOD(GattCharacteristic::PropertyFlags, CharacteristicProperties, (), (const, override));
+    };
+
     class GattClientCharacteristicOperationsMock
         : public services::GattClientCharacteristicOperations
     {
     public:
-        MOCK_METHOD(void, Read, (const GattClientCharacteristicOperationsObserver&, const infra::Function<void(const infra::ConstByteRange&)>&), (const, override));
-        MOCK_METHOD(void, Write, (const GattClientCharacteristicOperationsObserver&, infra::ConstByteRange, const infra::Function<void()>&), (const, override));
-        MOCK_METHOD(void, WriteWithoutResponse, (const GattClientCharacteristicOperationsObserver&, infra::ConstByteRange), (const, override));
-
-        MOCK_METHOD(void, EnableNotification, (const GattClientCharacteristicOperationsObserver&, const infra::Function<void()>&), (const, override));
-        MOCK_METHOD(void, DisableNotification, (const GattClientCharacteristicOperationsObserver&, const infra::Function<void()>&), (const, override));
-        MOCK_METHOD(void, EnableIndication, (const GattClientCharacteristicOperationsObserver&, const infra::Function<void()>&), (const, override));
-        MOCK_METHOD(void, DisableIndication, (const GattClientCharacteristicOperationsObserver&, const infra::Function<void()>&), (const, override));
+        MOCK_METHOD(void, Read, (const GattClientCharacteristicOperationsObserver&, const infra::Function<void(const infra::ConstByteRange&)>&), (override));
+        MOCK_METHOD(void, Write, (const GattClientCharacteristicOperationsObserver&, infra::ConstByteRange, const infra::Function<void()>&), (override));
+        MOCK_METHOD(void, WriteWithoutResponse, (const GattClientCharacteristicOperationsObserver&, infra::ConstByteRange), (override));
+
+        MOCK_METHOD(void, EnableNotification, (const GattClientCharacteristicOperationsObserver&, const infra::Function<void()>&), (override));
+        MOCK_METHOD(void, DisableNotification, (const GattClientCharacteristicOperationsObserver&, const infra::Function<void()>&), (override));
+        MOCK_METHOD(void, EnableIndication, (const GattClientCharacteristicOperationsObserver&, const infra::Function<void()>&), (override));
+        MOCK_METHOD(void, DisableIndication, (const GattClientCharacteristicOperationsObserver&, const infra::Function<void()>&), (override));
     };
 
     class GattClientDiscoveryObserverMock
@@ -48,8 +56,8 @@ namespace services
     {
     public:
         MOCK_METHOD(void, StartServiceDiscovery, (), (override));
-        MOCK_METHOD(void, StartCharacteristicDiscovery, (const GattService& service), (override));
-        MOCK_METHOD(void, StartDescriptorDiscovery, (const GattService& service), (override));
+        MOCK_METHOD(void, StartCharacteristicDiscovery, (AttAttribute::Handle handle, AttAttribute::Handle endHandle), (override));
+        MOCK_METHOD(void, StartDescriptorDiscovery, (AttAttribute::Handle handle, AttAttribute::Handle endHandle), (override));
     };
 }
 

services/cucumber/test/TestCucumberWireProtocolServer.cpp

@@ -86,7 +86,7 @@ GIVEN("nothing happens for %d seconds")
         infra::StringInputStream secondsStream(secondsString);
         uint32_t seconds;
         secondsStream >> seconds;
-        Context().TimeoutTimer().Start(std::chrono::seconds(seconds), [=]()
+        Context().TimeoutTimer().Start(std::chrono::seconds(seconds), [this]()
             {
                 Success();
             });