fixing Set on report properties
Setting valid value for property which is incorrect for current
configuration will result in OK response, but will change ErrorMessages
property. ErrorMessages will contain information about error type and
property name.
Tested:
- Change interval from 1000 to 0 for periodic report changes status from
Enabled to Disabled.
- Change interval from 0 to 1000 for periodic report changes status from
Disabled to Enabled.
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Signed-off-by: Szymon Dompke <szymon.dompke@intel.com>
Change-Id: I34add1ed0308b3da0850b1db6a67a217d11b6956
diff --git a/src/report.cpp b/src/report.cpp
index 540418a..be98c6d 100644
--- a/src/report.cpp
+++ b/src/report.cpp
@@ -39,7 +39,7 @@
deduceBufferSize(reportUpdates, reportingType)),
objServer(objServer), metrics(std::move(metricsIn)), timer(ioc),
triggerIds(collectTriggerIds(ioc)), reportStorage(reportStorageIn),
- enabled(enabledIn), clock(std::move(clock)), messanger(ioc)
+ clock(std::move(clock)), messanger(ioc)
{
readingParameters =
toReadingParameters(utils::transform(metrics, [](const auto& metric) {
@@ -75,18 +75,12 @@
});
});
- persistency = storeConfiguration();
+ errorMessages = verify();
+ state.set<ReportFlags::enabled, ReportFlags::valid>(enabledIn,
+ errorMessages.empty());
+
reportIface = makeReportInterface(reportFactory);
-
- updateReportingType(reportingType);
-
- if (enabled)
- {
- for (auto& metric : this->metrics)
- {
- metric->initialize();
- }
- }
+ persistency = storeConfiguration();
messanger.on_receive<messages::TriggerPresenceChangedInd>(
[this](const auto& msg) {
@@ -137,6 +131,37 @@
}
}
+void Report::activate()
+{
+ for (auto& metric : this->metrics)
+ {
+ metric->initialize();
+ }
+
+ scheduleTimer();
+
+ if (reportIface)
+ {
+ reportIface->signal_property("errors");
+ }
+}
+
+void Report::deactivate()
+{
+ for (auto& metric : metrics)
+ {
+ metric->deinitialize();
+ }
+
+ unregisterFromMetrics = nullptr;
+ timer.cancel();
+
+ if (reportIface)
+ {
+ reportIface->signal_property("Errors");
+ }
+}
+
uint64_t Report::getSensorCount(
const std::vector<std::shared_ptr<interfaces::Metric>>& metrics)
{
@@ -200,70 +225,71 @@
{
auto dbusIface =
objServer->add_unique_interface(getPath(), reportIfaceName);
- dbusIface->register_property_rw(
- "Enabled", enabled, sdbusplus::vtable::property_::emits_change,
- [this](bool newVal, const auto&) {
- if (newVal != enabled)
+ dbusIface->register_property_rw<bool>(
+ "Enabled", sdbusplus::vtable::property_::emits_change,
+ [this](bool newVal, auto& oldValue) {
+ if (newVal != state.get<ReportFlags::enabled>())
{
- if (true == newVal && ReportingType::periodic == reportingType)
- {
- scheduleTimerForPeriodicReport(interval);
- }
- if (newVal)
- {
- for (auto& metric : metrics)
- {
- metric->initialize();
- }
- }
- else
- {
- for (auto& metric : metrics)
- {
- metric->deinitialize();
- }
- }
+ state.set<ReportFlags::enabled>(oldValue = newVal);
- enabled = newVal;
persistency = storeConfiguration();
}
return 1;
},
- [this](const auto&) { return enabled; });
- dbusIface->register_property_rw(
- "Interval", interval.count(),
- sdbusplus::vtable::property_::emits_change,
- [this](uint64_t newVal, auto&) {
- if (Milliseconds newValT{newVal};
- newValT >= ReportManager::minInterval)
+ [this](const auto&) { return state.get<ReportFlags::enabled>(); });
+ dbusIface->register_property_r<
+ std::vector<std::tuple<std::string, std::string>>>(
+ "ErrorMessages", sdbusplus::vtable::property_::emits_change,
+ [this](const auto&) {
+ return utils::transform(errorMessages, [](const auto& em) {
+ return std::tuple<std::string, std::string>(
+ utils::enumToString(em.error), em.arg0);
+ });
+ });
+ dbusIface->register_property_rw<uint64_t>(
+ "Interval", sdbusplus::vtable::property_::emits_change,
+ [this](uint64_t newVal, auto& oldVal) {
+ const Milliseconds newValT{newVal};
+ if (newValT < ReportManager::minInterval &&
+ newValT != Milliseconds{0})
{
- if (newValT != interval)
- {
- interval = newValT;
- persistency = storeConfiguration();
- }
- return 1;
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument),
+ "Invalid interval");
}
- throw sdbusplus::exception::SdBusError(
- static_cast<int>(std::errc::invalid_argument),
- "Invalid interval");
+
+ if (newValT != interval)
+ {
+ oldVal = newVal;
+ interval = newValT;
+
+ errorMessages = verify();
+ if (state.set<ReportFlags::valid>(errorMessages.empty()) ==
+ StateEvent::active)
+ {
+ scheduleTimer();
+ }
+
+ persistency = storeConfiguration();
+ }
+ return 1;
},
[this](const auto&) { return interval.count(); });
- dbusIface->register_property_rw(
- "Persistency", persistency, sdbusplus::vtable::property_::emits_change,
- [this](bool newVal, const auto&) {
+ dbusIface->register_property_rw<bool>(
+ "Persistency", sdbusplus::vtable::property_::emits_change,
+ [this](bool newVal, auto& oldVal) {
if (newVal == persistency)
{
return 1;
}
if (newVal)
{
- persistency = storeConfiguration();
+ persistency = oldVal = storeConfiguration();
}
else
{
reportStorage.remove(reportFileName());
- persistency = false;
+ persistency = oldVal = false;
}
return 1;
},
@@ -272,27 +298,25 @@
dbusIface->register_property_r("Readings", readings,
sdbusplus::vtable::property_::emits_change,
[this](const auto&) { return readings; });
- dbusIface->register_property_rw(
- "ReportingType", std::string(),
- sdbusplus::vtable::property_::emits_change,
+ dbusIface->register_property_rw<std::string>(
+ "ReportingType", sdbusplus::vtable::property_::emits_change,
[this](auto newVal, auto& oldVal) {
ReportingType tmp = utils::toReportingType(newVal);
if (tmp != reportingType)
{
- if (tmp == ReportingType::periodic)
+ reportingType = tmp;
+ oldVal = std::move(newVal);
+
+ errorMessages = verify();
+ if (state.set<ReportFlags::valid>(errorMessages.empty()) ==
+ StateEvent::active)
{
- if (interval < ReportManager::minInterval)
- {
- throw sdbusplus::exception::SdBusError(
- static_cast<int>(std::errc::invalid_argument),
- "Invalid interval");
- }
+ scheduleTimer();
}
- updateReportingType(tmp);
- setReadingBuffer(reportUpdates);
persistency = storeConfiguration();
- oldVal = std::move(newVal);
+
+ setReadingBuffer(reportUpdates);
}
return 1;
},
@@ -305,7 +329,8 @@
"ReadingParametersFutureVersion", readingParameters,
sdbusplus::vtable::property_::emits_change,
[this, &reportFactory](auto newVal, auto& oldVal) {
- reportFactory.updateMetrics(metrics, enabled, newVal);
+ reportFactory.updateMetrics(
+ metrics, state.get<ReportFlags::enabled>(), newVal);
readingParameters = toReadingParameters(
utils::transform(metrics, [](const auto& metric) {
return metric->dumpConfiguration();
@@ -315,23 +340,22 @@
return 1;
},
[this](const auto&) { return readingParameters; });
- dbusIface->register_property_r(
- "EmitsReadingsUpdate", bool{}, sdbusplus::vtable::property_::none,
+ dbusIface->register_property_r<bool>(
+ "EmitsReadingsUpdate", sdbusplus::vtable::property_::none,
[this](const auto&) {
return reportActions.contains(ReportAction::emitsReadingsUpdate);
});
- dbusIface->register_property_r("Name", std::string{},
- sdbusplus::vtable::property_::const_,
- [this](const auto&) { return name; });
- dbusIface->register_property_r(
- "LogToMetricReportsCollection", bool{},
- sdbusplus::vtable::property_::const_, [this](const auto&) {
+ dbusIface->register_property_r<std::string>(
+ "Name", sdbusplus::vtable::property_::const_,
+ [this](const auto&) { return name; });
+ dbusIface->register_property_r<bool>(
+ "LogToMetricReportsCollection", sdbusplus::vtable::property_::const_,
+ [this](const auto&) {
return reportActions.contains(
ReportAction::logToMetricReportsCollection);
});
- dbusIface->register_property_rw(
- "ReportActions", std::vector<std::string>{},
- sdbusplus::vtable::property_::emits_change,
+ dbusIface->register_property_rw<std::vector<std::string>>(
+ "ReportActions", sdbusplus::vtable::property_::emits_change,
[this](auto newVal, auto& oldVal) {
auto tmp = utils::transform<std::unordered_set>(
newVal, [](const auto& reportAction) {
@@ -420,11 +444,6 @@
void Report::scheduleTimerForPeriodicReport(Milliseconds timerInterval)
{
- if (!enabled)
- {
- return;
- }
-
timer.expires_after(timerInterval);
timer.async_wait([this](boost::system::error_code ec) {
timerProcForPeriodicReport(ec, *this);
@@ -433,11 +452,6 @@
void Report::scheduleTimerForOnChangeReport()
{
- if (!enabled)
- {
- return;
- }
-
constexpr Milliseconds timerInterval{100};
timer.expires_after(timerInterval);
@@ -448,7 +462,7 @@
void Report::updateReadings()
{
- if (!enabled)
+ if (!state.isActive())
{
return;
}
@@ -461,17 +475,19 @@
for (const auto& metric : metrics)
{
+ if (!state.isActive())
+ {
+ break;
+ }
+
for (const auto& [id, metadata, value, timestamp] :
metric->getUpdatedReadings())
{
if (reportUpdates == ReportUpdates::appendStopsWhenFull &&
readingsBuffer.isFull())
{
- enabled = false;
- for (auto& m : metrics)
- {
- m->deinitialize();
- }
+ state.set<ReportFlags::enabled>(false);
+ reportIface->signal_property("Enabled");
break;
}
readingsBuffer.emplace(id, metadata, value, timestamp);
@@ -500,7 +516,7 @@
{
nlohmann::json data;
- data["Enabled"] = enabled;
+ data["Enabled"] = state.get<ReportFlags::enabled>();
data["Version"] = reportVersion;
data["Id"] = id;
data["Name"] = name;
@@ -568,37 +584,37 @@
updateReadings();
}
-void Report::updateReportingType(ReportingType newReportingType)
+void Report::scheduleTimer()
{
- if (reportingType != newReportingType)
- {
- timer.cancel();
- unregisterFromMetrics = nullptr;
- }
-
- reportingType = newReportingType;
-
switch (reportingType)
{
case ReportingType::periodic:
{
+ unregisterFromMetrics = nullptr;
scheduleTimerForPeriodicReport(interval);
break;
}
case ReportingType::onChange:
{
- unregisterFromMetrics = [this] {
+ if (!unregisterFromMetrics)
+ {
+ unregisterFromMetrics = [this] {
+ for (auto& metric : metrics)
+ {
+ metric->unregisterFromUpdates(*this);
+ }
+ };
+
for (auto& metric : metrics)
{
- metric->unregisterFromUpdates(*this);
+ metric->registerForUpdates(*this);
}
- };
+ }
bool isTimerRequired = false;
for (auto& metric : metrics)
{
- metric->registerForUpdates(*this);
if (metric->isTimerRequired())
{
isTimerRequired = true;
@@ -609,9 +625,31 @@
{
scheduleTimerForOnChangeReport();
}
+ else
+ {
+ timer.cancel();
+ }
break;
}
default:
+ unregisterFromMetrics = nullptr;
+ timer.cancel();
break;
}
}
+
+std::vector<ErrorMessage> Report::verify() const
+{
+ std::vector<ErrorMessage> result;
+
+ if ((reportingType == ReportingType::periodic &&
+ interval == Milliseconds{0}) ||
+ (reportingType != ReportingType::periodic &&
+ interval != Milliseconds{0}))
+ {
+ result.emplace_back(ErrorType::propertyConflict, "Interval");
+ result.emplace_back(ErrorType::propertyConflict, "ReportingType");
+ }
+
+ return result;
+}
diff --git a/src/report.hpp b/src/report.hpp
index 6d8c6b4..7609297 100644
--- a/src/report.hpp
+++ b/src/report.hpp
@@ -7,6 +7,8 @@
#include "interfaces/report.hpp"
#include "interfaces/report_factory.hpp"
#include "interfaces/report_manager.hpp"
+#include "state.hpp"
+#include "types/error_message.hpp"
#include "types/readings.hpp"
#include "types/report_action.hpp"
#include "types/report_types.hpp"
@@ -81,6 +83,9 @@
void metricUpdated() override;
+ void activate();
+ void deactivate();
+
private:
std::unique_ptr<sdbusplus::asio::dbus_interface>
makeReportInterface(const interfaces::ReportFactory& reportFactory);
@@ -104,7 +109,8 @@
bool storeConfiguration() const;
bool shouldStoreMetricValues() const;
void updateReadings();
- void updateReportingType(ReportingType);
+ void scheduleTimer();
+ std::vector<ErrorMessage> verify() const;
std::string id;
std::string name;
@@ -127,11 +133,13 @@
std::unordered_set<std::string> triggerIds;
interfaces::JsonStorage& reportStorage;
- bool enabled;
std::unique_ptr<interfaces::Clock> clock;
utils::Messanger messanger;
std::optional<OnChangeContext> onChangeContext;
utils::Ensure<std::function<void()>> unregisterFromMetrics;
+ State<ReportFlags, Report, ReportFlags::enabled, ReportFlags::valid> state{
+ *this};
+ std::vector<ErrorMessage> errorMessages;
public:
static constexpr const char* reportIfaceName =
diff --git a/src/report_manager.cpp b/src/report_manager.cpp
index 926b642..738dd6d 100644
--- a/src/report_manager.cpp
+++ b/src/report_manager.cpp
@@ -154,7 +154,9 @@
"Append limit out of range");
}
- if (reportingType == ReportingType::periodic && interval < minInterval)
+ if ((reportingType == ReportingType::periodic && interval < minInterval) ||
+ (reportingType != ReportingType::periodic &&
+ interval != Milliseconds{0}))
{
throw sdbusplus::exception::SdBusError(
static_cast<int>(std::errc::invalid_argument), "Invalid interval");
diff --git a/src/state.hpp b/src/state.hpp
new file mode 100644
index 0000000..91a7daf
--- /dev/null
+++ b/src/state.hpp
@@ -0,0 +1,82 @@
+#pragma once
+
+#include <bitset>
+
+enum class ReportFlags
+{
+ enabled,
+ valid
+};
+
+enum class StateEvent
+{
+ active,
+ inactive,
+ activated,
+ deactivated
+};
+
+template <class T>
+concept boolean = std::is_same_v<T, bool>;
+
+template <class Flags, class Object, Flags... Keys>
+class State
+{
+ public:
+ explicit State(Object& object) : object(object)
+ {}
+
+ template <Flags... Indexes>
+ StateEvent set(boolean auto... values)
+ {
+ static_assert(sizeof...(Indexes) == sizeof...(values));
+
+ const bool previous = flags.all();
+
+ (flags.set(indexOf<0, Indexes, Keys...>(), values), ...);
+
+ if (previous != flags.all())
+ {
+ if (flags.all())
+ {
+ object.activate();
+ return StateEvent::activated;
+ }
+ else
+ {
+ object.deactivate();
+ return StateEvent::deactivated;
+ }
+ }
+
+ return flags.all() ? StateEvent::active : StateEvent::inactive;
+ }
+
+ template <Flags Index>
+ bool get() const
+ {
+ return flags.test(indexOf<0, Index, Keys...>());
+ }
+
+ bool isActive() const
+ {
+ return flags.all();
+ }
+
+ private:
+ template <size_t Index, Flags flag, Flags it, Flags... list>
+ static constexpr size_t indexOf()
+ {
+ if constexpr (flag == it)
+ {
+ return Index;
+ }
+ else
+ {
+ return indexOf<Index + 1, flag, list...>();
+ }
+ }
+
+ Object& object;
+ std::bitset<sizeof...(Keys)> flags{0};
+};
diff --git a/src/types/error_message.hpp b/src/types/error_message.hpp
new file mode 100644
index 0000000..de27772
--- /dev/null
+++ b/src/types/error_message.hpp
@@ -0,0 +1,11 @@
+#pragma once
+
+#include "error_type.hpp"
+
+#include <string>
+
+struct ErrorMessage
+{
+ ErrorType error;
+ std::string arg0;
+};
diff --git a/src/types/error_type.hpp b/src/types/error_type.hpp
new file mode 100644
index 0000000..0f1d61e
--- /dev/null
+++ b/src/types/error_type.hpp
@@ -0,0 +1,40 @@
+#pragma once
+
+#include "utils/conversion.hpp"
+
+#include <sdbusplus/exception.hpp>
+
+#include <array>
+#include <cstdint>
+#include <string_view>
+#include <type_traits>
+
+enum class ErrorType : uint32_t
+{
+ propertyConflict
+};
+
+namespace utils
+{
+
+constexpr auto convDataErrorType =
+ std::array{std::make_pair<std::string_view, ErrorType>(
+ "PropertyConflict", ErrorType::propertyConflict)};
+
+inline ErrorType toErrorType(std::underlying_type_t<ErrorType> value)
+{
+ return toEnum<ErrorType, minEnumValue(convDataErrorType),
+ maxEnumValue(convDataErrorType)>(value);
+}
+
+inline ErrorType toErrorType(const std::string& value)
+{
+ return toEnum(convDataErrorType, value);
+}
+
+inline std::string enumToString(ErrorType value)
+{
+ return std::string(enumToString(convDataErrorType, value));
+}
+
+} // namespace utils
diff --git a/src/utils/ensure.hpp b/src/utils/ensure.hpp
index cbe69c5..8f266ab 100644
--- a/src/utils/ensure.hpp
+++ b/src/utils/ensure.hpp
@@ -53,6 +53,11 @@
Ensure& operator=(const Ensure&) = delete;
+ explicit operator bool() const
+ {
+ return static_cast<bool>(functor);
+ }
+
private:
void clear()
{
diff --git a/tests/src/params/report_params.hpp b/tests/src/params/report_params.hpp
index db83b0d..3d32182 100644
--- a/tests/src/params/report_params.hpp
+++ b/tests/src/params/report_params.hpp
@@ -122,10 +122,10 @@
private:
std::string reportIdProperty = "TestId";
std::string reportNameProperty = "TestReport";
- ReportingType reportingTypeProperty = ReportingType::onRequest;
+ ReportingType reportingTypeProperty = ReportingType::onChange;
std::vector<ReportAction> reportActionsProperty = {
ReportAction::logToMetricReportsCollection};
- Milliseconds intervalProperty = ReportManager::minInterval;
+ Milliseconds intervalProperty{};
uint64_t appendLimitProperty = 123;
ReportUpdates reportUpdatesProperty = ReportUpdates::overwrite;
std::vector<LabeledMetricParameters> metricParametersProperty{
diff --git a/tests/src/test_report.cpp b/tests/src/test_report.cpp
index 01f4853..8226f66 100644
--- a/tests/src/test_report.cpp
+++ b/tests/src/test_report.cpp
@@ -25,6 +25,9 @@
using namespace std::chrono_literals;
namespace tstring = utils::tstring;
+using ErrorMessageDbusType = std::tuple<std::string, std::string>;
+using ErrorMessagesDbusType = std::vector<ErrorMessageDbusType>;
+
constexpr Milliseconds systemTimestamp = 55ms;
namespace
@@ -148,6 +151,26 @@
property, newValue);
}
+ template <class T>
+ struct ChangePropertyParams
+ {
+ Matcher<T> valueBefore = _;
+ T newValue;
+ Matcher<boost::system::error_code> ec =
+ Eq(boost::system::errc::success);
+ Matcher<T> valueAfter = Eq(newValue);
+ };
+
+ template <class T>
+ static void changeProperty(const std::string& path,
+ const std::string& property,
+ ChangePropertyParams<T> p)
+ {
+ ASSERT_THAT(getProperty<T>(path, property), p.valueBefore);
+ ASSERT_THAT(setProperty<T>(path, property, p.newValue), p.ec);
+ EXPECT_THAT(getProperty<T>(path, property), p.valueAfter);
+ }
+
boost::system::error_code call(const std::string& path,
const std::string& interface,
const std::string& method)
@@ -170,6 +193,13 @@
{
return call(path, Report::deleteIfaceName, "Delete");
}
+
+ static std::pair<std::string, std::vector<std::string>>
+ makeStateDetail(const std::string& detailType,
+ std::vector<std::string> detailArgs)
+ {
+ return make_pair(detailType, detailArgs);
+ }
};
TEST_F(TestReport, returnsId)
@@ -209,7 +239,10 @@
Eq(defaultParams().reportName()));
EXPECT_THAT(
getProperty<std::vector<std::string>>(sut->getPath(), "TriggerIds"),
- Eq(std::vector<std::string>()));
+ IsEmpty());
+ EXPECT_THAT(
+ getProperty<ErrorMessagesDbusType>(sut->getPath(), "ErrorMessages"),
+ IsEmpty());
}
TEST_F(TestReport, readingsAreInitialyEmpty)
@@ -244,25 +277,23 @@
TEST_F(TestReport, setReportingTypeWithValidNewType)
{
- std::string newType = "Periodic";
- std::string currType = utils::enumToString(defaultParams().reportingType());
-
- EXPECT_THAT(newType, Ne(currType));
- EXPECT_THAT(setProperty(sut->getPath(), "ReportingType", newType).value(),
- Eq(boost::system::errc::success));
- EXPECT_THAT(getProperty<std::string>(sut->getPath(), "ReportingType"),
- Eq(newType));
+ changeProperty<std::string>(
+ sut->getPath(), "ReportingType",
+ {.valueBefore = Not(Eq(utils::enumToString(ReportingType::onRequest))),
+ .newValue = utils::enumToString(ReportingType::onRequest)});
}
TEST_F(TestReport, setReportingTypeWithInvalidType)
{
- std::string newType = "Periodic_ABC";
- std::string prevType = utils::enumToString(defaultParams().reportingType());
+ const std::string currentValue =
+ utils::enumToString(defaultParams().reportingType());
- EXPECT_THAT(setProperty(sut->getPath(), "ReportingType", newType).value(),
- Eq(boost::system::errc::invalid_argument));
- EXPECT_THAT(getProperty<std::string>(sut->getPath(), "ReportingType"),
- Eq(prevType));
+ changeProperty<std::string>(
+ sut->getPath(), "ReportingType",
+ {.valueBefore = Eq(currentValue),
+ .newValue = "Periodic_ABC",
+ .ec = Eq(boost::system::errc::invalid_argument),
+ .valueAfter = Eq(currentValue)});
}
TEST_F(TestReport, setReportActionsWithValidNewActions)
@@ -369,7 +400,7 @@
TEST_F(TestReport, setIntervalWithValidValue)
{
- uint64_t newValue = defaultParams().interval().count() + 1;
+ uint64_t newValue = ReportManager::minInterval.count() * 42;
EXPECT_THAT(setProperty(sut->getPath(), "Interval", newValue).value(),
Eq(boost::system::errc::success));
EXPECT_THAT(getProperty<uint64_t>(sut->getPath(), "Interval"),
@@ -380,13 +411,102 @@
TestReport,
settingIntervalWithInvalidValueDoesNotChangePropertyAndReturnsInvalidArgument)
{
- uint64_t newValue = defaultParams().interval().count() - 1;
+ uint64_t newValue = ReportManager::minInterval.count() - 1;
EXPECT_THAT(setProperty(sut->getPath(), "Interval", newValue).value(),
Eq(boost::system::errc::invalid_argument));
EXPECT_THAT(getProperty<uint64_t>(sut->getPath(), "Interval"),
Eq(defaultParams().interval().count()));
}
+TEST_F(TestReport, settingInvalidReportingTypeCreatesErrorMessage)
+{
+ auto report = makeReport(defaultParams()
+ .reportId("report2")
+ .reportingType(ReportingType::onRequest)
+ .interval(Milliseconds{0}));
+
+ EXPECT_THAT(
+ setProperty<std::string>(report->getPath(), "ReportingType", "Periodic")
+ .value(),
+ Eq(boost::system::errc::success));
+
+ EXPECT_THAT(getProperty<std::string>(report->getPath(), "ReportingType"),
+ Eq("Periodic"));
+ EXPECT_THAT(
+ getProperty<ErrorMessagesDbusType>(report->getPath(), "ErrorMessages"),
+ UnorderedElementsAre(
+ ErrorMessageDbusType(
+ utils::enumToString(ErrorType::propertyConflict), "Interval"),
+ ErrorMessageDbusType(
+ utils::enumToString(ErrorType::propertyConflict),
+ "ReportingType")));
+}
+
+TEST_F(TestReport, settingValidReportingTypeRemovesErrors)
+{
+ auto report = makeReport(defaultParams()
+ .reportId("report2")
+ .reportingType(ReportingType::onRequest)
+ .interval(Milliseconds{0}));
+
+ EXPECT_THAT(
+ setProperty<std::string>(report->getPath(), "ReportingType", "Periodic")
+ .value(),
+ Eq(boost::system::errc::success));
+ EXPECT_THAT(setProperty<std::string>(report->getPath(), "ReportingType",
+ "OnRequest")
+ .value(),
+ Eq(boost::system::errc::success));
+
+ EXPECT_THAT(getProperty<std::string>(report->getPath(), "ReportingType"),
+ Eq("OnRequest"));
+ EXPECT_THAT(
+ getProperty<ErrorMessagesDbusType>(report->getPath(), "ErrorMessages"),
+ IsEmpty());
+}
+
+TEST_F(TestReport, settingInvalidIntervalDisablesReport)
+{
+ auto report = makeReport(defaultParams()
+ .reportId("report2")
+ .reportingType(ReportingType::periodic)
+ .interval(ReportManager::minInterval));
+
+ EXPECT_THAT(setProperty<uint64_t>(report->getPath(), "Interval", 0).value(),
+ Eq(boost::system::errc::success));
+
+ EXPECT_THAT(getProperty<uint64_t>(report->getPath(), "Interval"), Eq(0u));
+ EXPECT_THAT(
+ getProperty<ErrorMessagesDbusType>(report->getPath(), "ErrorMessages"),
+ UnorderedElementsAre(
+ ErrorMessageDbusType(
+ utils::enumToString(ErrorType::propertyConflict), "Interval"),
+ ErrorMessageDbusType(
+ utils::enumToString(ErrorType::propertyConflict),
+ "ReportingType")));
+}
+
+TEST_F(TestReport, settingValidIntervalEnablesReport)
+{
+ auto report = makeReport(defaultParams()
+ .reportId("report2")
+ .reportingType(ReportingType::periodic)
+ .interval(ReportManager::minInterval));
+
+ EXPECT_THAT(setProperty<uint64_t>(report->getPath(), "Interval", 0).value(),
+ Eq(boost::system::errc::success));
+ EXPECT_THAT(setProperty<uint64_t>(report->getPath(), "Interval",
+ ReportManager::minInterval.count())
+ .value(),
+ Eq(boost::system::errc::success));
+
+ EXPECT_THAT(getProperty<uint64_t>(report->getPath(), "Interval"),
+ Eq(ReportManager::minInterval.count()));
+ EXPECT_THAT(
+ getProperty<ErrorMessagesDbusType>(report->getPath(), "ErrorMessages"),
+ IsEmpty());
+}
+
TEST_F(TestReport, settingEmitsReadingsUpdateHaveNoEffect)
{
EXPECT_THAT(
@@ -651,7 +771,9 @@
_, TestReportAllReportTypes,
Values(defaultParams().reportingType(ReportingType::onRequest),
defaultParams().reportingType(ReportingType::onChange),
- defaultParams().reportingType(ReportingType::periodic)));
+ defaultParams()
+ .reportingType(ReportingType::periodic)
+ .interval(ReportManager::minInterval)));
TEST_P(TestReportAllReportTypes, returnPropertValueOfReportType)
{
@@ -778,8 +900,9 @@
{
void SetUp() override
{
- sut =
- makeReport(defaultParams().reportingType(ReportingType::periodic));
+ sut = makeReport(defaultParams()
+ .reportingType(ReportingType::periodic)
+ .interval(ReportManager::minInterval));
}
};
@@ -1040,16 +1163,16 @@
InvokeWithoutArgs(DbusEnvironment::setPromise("readingsUpdated")));
const auto elapsed = DbusEnvironment::measureTime([this] {
- sut =
- makeReport(defaultParams()
- .reportingType(ReportingType::periodic)
- .reportActions({ReportAction::emitsReadingsUpdate}));
+ sut = makeReport(defaultParams()
+ .reportingType(ReportingType::periodic)
+ .reportActions({ReportAction::emitsReadingsUpdate})
+ .interval(ReportManager::minInterval));
makeMonitor();
EXPECT_TRUE(DbusEnvironment::waitForFuture("readingsUpdated"));
});
- EXPECT_THAT(elapsed, AllOf(Ge(defaultParams().interval()),
- Lt(defaultParams().interval() * 2)));
+ EXPECT_THAT(elapsed, AllOf(Ge(ReportManager::minInterval),
+ Lt(ReportManager::minInterval * 2)));
}
TEST_F(TestReportInitialization,
diff --git a/tests/src/test_report_manager.cpp b/tests/src/test_report_manager.cpp
index 55b5a3c..5d0d4a5 100644
--- a/tests/src/test_report_manager.cpp
+++ b/tests/src/test_report_manager.cpp
@@ -205,7 +205,7 @@
.Times(0);
reportParams.reportingType(ReportingType::periodic);
- reportParams.interval(reportParams.interval() - 1ms);
+ reportParams.interval(ReportManager::minInterval - 1ms);
auto [ec, path] = addReport(reportParams);