Remove retry "GetProperty" on fail
Removed retry "GetProperty" on fail when sensor is not present.
Monitoring PropertiesChanged signal is enough to get the latest
value from sensor when it is added to dbus environment. Added
test that check if newly added properties to dbus environment
send PropertiesChanged signal.
Tested: - All unit tests passed
Change-Id: Ibe92b23dd062a6d48b0b6d5bc0dc2198bcfa7a5e
Signed-off-by: Wludzik, Jozef <jozef.wludzik@intel.com>
diff --git a/src/sensor.cpp b/src/sensor.cpp
index d38fbd9..c4973cd 100644
--- a/src/sensor.cpp
+++ b/src/sensor.cpp
@@ -1,14 +1,11 @@
#include "sensor.hpp"
-#include "utils/detached_timer.hpp"
-
#include <boost/container/flat_map.hpp>
#include <phosphor-logging/log.hpp>
#include <sdbusplus/asio/property.hpp>
#include <sdbusplus/bus/match.hpp>
#include <functional>
-#include <iostream>
Sensor::Sensor(interfaces::Sensor::Id sensorId, boost::asio::io_context& ioc,
const std::shared_ptr<sdbusplus::asio::connection>& bus) :
@@ -40,24 +37,10 @@
"xyz.openbmc_project.Sensor.Value", "Value",
[lock, id = sensorId,
weakSelf = weak_from_this()](boost::system::error_code ec) {
- phosphor::logging::log<phosphor::logging::level::ERR>(
+ phosphor::logging::log<phosphor::logging::level::WARNING>(
"DBus 'GetProperty' call failed on Sensor Value",
phosphor::logging::entry("sensor=%s, ec=%lu", id.str().c_str(),
ec.value()));
-
- if (auto self = weakSelf.lock())
- {
-
- constexpr auto retryIntervalAfterFailedRead =
- std::chrono::seconds(30);
- utils::makeDetachedTimer(
- self->ioc, retryIntervalAfterFailedRead, [weakSelf] {
- if (auto self = weakSelf.lock())
- {
- self->async_read();
- }
- });
- }
},
[lock, weakSelf = weak_from_this()](double newValue) {
if (auto self = weakSelf.lock())
diff --git a/src/utils/detached_timer.hpp b/src/utils/detached_timer.hpp
index 21343f9..224e5a1 100644
--- a/src/utils/detached_timer.hpp
+++ b/src/utils/detached_timer.hpp
@@ -1,7 +1,6 @@
#pragma once
#include <boost/asio/io_context.hpp>
-#include <boost/asio/spawn.hpp>
#include <boost/asio/steady_timer.hpp>
#include <chrono>
diff --git a/tests/src/stubs/dbus_sensor_object.cpp b/tests/src/stubs/dbus_sensor_object.cpp
index c3d4170..9d6f185 100644
--- a/tests/src/stubs/dbus_sensor_object.cpp
+++ b/tests/src/stubs/dbus_sensor_object.cpp
@@ -15,18 +15,13 @@
ioc(ioc),
bus(bus), objServer(objServer)
{
- sensorIface = objServer->add_interface(path(), interface());
-
- sensorIface->register_property_r(property.value(), double{},
- sdbusplus::vtable::property_::emits_change,
- [this](const auto&) { return value; });
-
- sensorIface->initialize();
-}
-
-DbusSensorObject::~DbusSensorObject()
-{
- objServer->remove_interface(sensorIface);
+ sensorIface = objServer->add_unique_interface(
+ path(), interface(), [this](auto& iface) {
+ iface.register_property_r(
+ property.value(), value,
+ sdbusplus::vtable::property_::emits_change,
+ [this](const auto&) { return value; });
+ });
}
void DbusSensorObject::setValue(double v)
diff --git a/tests/src/stubs/dbus_sensor_object.hpp b/tests/src/stubs/dbus_sensor_object.hpp
index 99271fa..45ee4c7 100644
--- a/tests/src/stubs/dbus_sensor_object.hpp
+++ b/tests/src/stubs/dbus_sensor_object.hpp
@@ -15,7 +15,6 @@
boost::asio::io_context& ioc,
const std::shared_ptr<sdbusplus::asio::connection>& bus,
const std::shared_ptr<sdbusplus::asio::object_server>& objServer);
- ~DbusSensorObject();
static const char* path();
static const char* interface();
@@ -35,7 +34,7 @@
std::shared_ptr<sdbusplus::asio::connection> bus;
std::shared_ptr<sdbusplus::asio::object_server> objServer;
- std::shared_ptr<sdbusplus::asio::dbus_interface> sensorIface;
+ std::unique_ptr<sdbusplus::asio::dbus_interface> sensorIface;
double value = 0.0;
};
diff --git a/tests/src/test_sensor.cpp b/tests/src/test_sensor.cpp
index 1398738..5339047 100644
--- a/tests/src/test_sensor.cpp
+++ b/tests/src/test_sensor.cpp
@@ -20,7 +20,7 @@
public:
void SetUp() override
{
- sensorObject.setValue(42.7);
+ sensorObject->setValue(42.7);
}
void TearDown() override
@@ -31,33 +31,36 @@
void
registerForUpdates(std::shared_ptr<interfaces::SensorListener> listener)
{
- DbusEnvironment::synchronizedPost(
- [this, listener] { sut->registerForUpdates(listener); });
+ sut->registerForUpdates(listener);
+ DbusEnvironment::synchronizeIoc();
}
- std::chrono::milliseconds notifiesInGivenIntervalAfterSchedule(
- std::chrono::milliseconds interval);
+ static std::unique_ptr<stubs::DbusSensorObject> makeSensorObject()
+ {
+ return std::make_unique<stubs::DbusSensorObject>(
+ DbusEnvironment::getIoc(), DbusEnvironment::getBus(),
+ DbusEnvironment::getObjServer());
+ }
- stubs::DbusSensorObject sensorObject{DbusEnvironment::getIoc(),
- DbusEnvironment::getBus(),
- DbusEnvironment::getObjServer()};
+ std::unique_ptr<stubs::DbusSensorObject> sensorObject = makeSensorObject();
SensorCache sensorCache;
uint64_t timestamp = std::time(0);
std::shared_ptr<Sensor> sut = sensorCache.makeSensor<Sensor>(
- DbusEnvironment::serviceName(), sensorObject.path(),
+ DbusEnvironment::serviceName(), sensorObject->path(),
DbusEnvironment::getIoc(), DbusEnvironment::getBus());
std::shared_ptr<SensorListenerMock> listenerMock =
std::make_shared<StrictMock<SensorListenerMock>>();
std::shared_ptr<SensorListenerMock> listenerMock2 =
std::make_shared<StrictMock<SensorListenerMock>>();
+ MockFunction<void()> checkPoint;
};
TEST_F(TestSensor, createsCorretlyViaSensorCache)
{
ASSERT_THAT(sut->id(),
Eq(Sensor::Id("Sensor", DbusEnvironment::serviceName(),
- sensorObject.path())));
+ sensorObject->path())));
}
TEST_F(TestSensor, notifiesWithValueAfterRegister)
@@ -110,40 +113,36 @@
EXPECT_CALL(*listenerMock, sensorUpdated(Ref(*sut), Ge(timestamp), 42.7))
.WillOnce(InvokeWithoutArgs(DbusEnvironment::setPromise("notify")));
- sensorObject.setValue(42.7);
+ sensorObject->setValue(42.7);
ASSERT_TRUE(DbusEnvironment::waitForFuture("notify"));
}
TEST_F(TestSensorNotification, notifiesListenerWithValueWhenNoChangeOccurs)
{
- Sequence seq;
+ InSequence seq;
- EXPECT_CALL(*listenerMock, sensorUpdated(Ref(*sut), Ge(timestamp), 42.7))
- .InSequence(seq);
+ EXPECT_CALL(*listenerMock, sensorUpdated(Ref(*sut), Ge(timestamp), 42.7));
EXPECT_CALL(*listenerMock, sensorUpdated(Ref(*sut), Ge(timestamp)))
- .InSequence(seq)
.WillOnce(InvokeWithoutArgs(DbusEnvironment::setPromise("notify")));
- sensorObject.setValue(42.7);
- sensorObject.setValue(42.7);
+ sensorObject->setValue(42.7);
+ sensorObject->setValue(42.7);
ASSERT_TRUE(DbusEnvironment::waitForFuture("notify"));
}
TEST_F(TestSensorNotification, doesntNotifyExpiredListener)
{
- Sequence seq;
- EXPECT_CALL(*listenerMock2, sensorUpdated(Ref(*sut), Ge(timestamp), 0.))
- .InSequence(seq);
+ InSequence seq;
+ EXPECT_CALL(*listenerMock2, sensorUpdated(Ref(*sut), Ge(timestamp), 0.));
EXPECT_CALL(*listenerMock2, sensorUpdated(Ref(*sut), Ge(timestamp), 42.7))
- .InSequence(seq)
.WillOnce(InvokeWithoutArgs(DbusEnvironment::setPromise("notify")));
registerForUpdates(listenerMock2);
listenerMock = nullptr;
- sensorObject.setValue(42.7);
+ sensorObject->setValue(42.7);
ASSERT_TRUE(DbusEnvironment::waitForFuture("notify"));
}
@@ -154,3 +153,28 @@
registerForUpdates(listenerMock2);
}
+
+TEST_F(TestSensorNotification,
+ dbusSensorIsAddedToSystemAfterSensorIsCreatedThenValueIsUpdated)
+{
+ InSequence seq;
+
+ EXPECT_CALL(*listenerMock,
+ sensorUpdated(Ref(*sut), Ge(timestamp), DoubleEq(42.7)))
+ .WillOnce(
+ InvokeWithoutArgs(DbusEnvironment::setPromise("notify-change")));
+ EXPECT_CALL(checkPoint, Call());
+ EXPECT_CALL(*listenerMock,
+ sensorUpdated(Ref(*sut), Ge(timestamp), DoubleEq(0.)))
+ .WillOnce(
+ InvokeWithoutArgs(DbusEnvironment::setPromise("notify-create")));
+
+ sensorObject->setValue(42.7);
+ DbusEnvironment::waitForFuture("notify-change");
+
+ checkPoint.Call();
+
+ sensorObject = nullptr;
+ sensorObject = makeSensorObject();
+ DbusEnvironment::waitForFuture("notify-create");
+}