[Merge] lp:~kaijanmaki/dbus-cpp/read-only-properties-changed-fix into lp:dbus-cpp

Thomas Voß thomas.voss at canonical.com
Wed Jun 4 08:35:13 UTC 2014


Review: Needs Fixing



Diff comments:

> === modified file 'include/core/dbus/impl/property.h'
> --- include/core/dbus/impl/property.h	2014-01-20 22:04:16 +0000
> +++ include/core/dbus/impl/property.h	2014-06-03 08:27:10 +0000
> @@ -161,10 +161,14 @@
>      try
>      {
>          auto value = arg.as<typename PropertyType::ValueType>();
> -        set(value);
> +        Super::set(value);
> +    }
> +    catch (std::exception &e){

Please catch by const reference.

> +        std::cout << __PRETTY_FUNCTION__ << ": " << e.what() << std::endl;
>      }
>      catch (...)
>      {
> +        std::cout << __PRETTY_FUNCTION__ << ": " << "Unknown exception." << std::endl;
>      }
>  }
>  }
> 
> === modified file 'tests/service_test.cpp'
> --- tests/service_test.cpp	2014-02-18 06:43:55 +0000
> +++ tests/service_test.cpp	2014-06-03 08:27:10 +0000
> @@ -82,11 +82,25 @@
>              auto writable_property = skeleton->get_property<test::Service::Properties::Dummy>();
>              writable_property->set(expected_value);
>  
> -            skeleton->install_method_handler<test::Service::Method>([bus, skeleton, expected_value](const dbus::Message::Ptr& msg)
> +            auto readonly_property = skeleton->get_property<test::Service::Properties::ReadOnly>();
> +            readonly_property->set(7);
> +
> +            skeleton->install_method_handler<test::Service::Method>([bus, skeleton, &readonly_property, expected_value](const dbus::Message::Ptr& msg)
>              {
>                  auto reply = dbus::Message::make_method_return(msg);
>                  reply->writer() << expected_value;
>                  bus->send(reply);
> +
> +                readonly_property->set(expected_value);
> +                auto changed_signal = skeleton->get_signal<core::dbus::interfaces::Properties::Signals::PropertiesChanged>();
> +                core::dbus::interfaces::Properties::Signals::PropertiesChanged::ArgumentType
> +                        args("this.is.unlikely.to.exist.Service",
> +                             {{test::Service::Properties::ReadOnly::name(),
> +                               core::dbus::types::TypedVariant<test::Service::Properties::ReadOnly::ValueType>(expected_value)},},

There seems to be a stray ',' present here.

> +                             {});
> +                skeleton->emit_signal<core::dbus::interfaces::Properties::Signals::PropertiesChanged, core::dbus::interfaces::Properties::Signals::PropertiesChanged::ArgumentType>(args);
> +                changed_signal->emit(args);
> +
>                  skeleton->emit_signal<test::Service::Signals::Dummy, int64_t>(expected_value);
>              });
>  
> @@ -118,6 +132,14 @@
>              {
>                  std::cout << "Dummy property changed: " << d << std::endl;
>              });
> +
> +            auto readonly_property = stub->get_property<test::Service::Properties::ReadOnly>();
> +            EXPECT_EQ(readonly_property->get(), 7);
> +            std::uint32_t changed_value = 0;
> +            readonly_property->changed().connect([&changed_value](std::uint32_t value){
> +                changed_value = value;
> +            });
> +
>              auto signal = stub->get_signal<test::Service::Signals::Dummy>();
>              int64_t received_signal_value = -1;
>              signal->connect([bus, &received_signal_value](const int32_t& value)
> @@ -144,6 +166,8 @@
>                  t.join();
>  
>              EXPECT_EQ(expected_value, received_signal_value);
> +            EXPECT_EQ(expected_value, readonly_property->get());
> +            EXPECT_EQ(changed_value, expected_value);
>  
>              return ::testing::Test::HasFailure() ? core::posix::exit::Status::failure : core::posix::exit::Status::success;
>          };
> 
> === modified file 'tests/test_service.h'
> --- tests/test_service.h	2014-03-05 06:58:15 +0000
> +++ tests/test_service.h	2014-06-03 08:27:10 +0000
> @@ -68,6 +68,18 @@
>              static const bool readable = true;
>              static const bool writable = true;
>          };
> +
> +        struct ReadOnly
> +        {
> +            inline static std::string name()
> +            {
> +                return "ReadOnly";
> +            };
> +            typedef Service Interface;
> +            typedef std::uint32_t ValueType;
> +            static const bool readable = true;
> +            static const bool writable = false;
> +        };
>      };
>  
>      struct Interfaces
> 


-- 
https://code.launchpad.net/~kaijanmaki/dbus-cpp/read-only-properties-changed-fix/+merge/221839
Your team Ubuntu Phablet Team is subscribed to branch lp:dbus-cpp.



More information about the Ubuntu-reviews mailing list