Allow disabling PID loops at runtime
<design concept>
Add the map of object enable interface to pid loops
in the zone then we can disable/enable each pid loop
process in a zone by dbus command.
[note]
Enable = true : enable process (default)
Enable = false : disable process
Tested:
In this case: we set Enable = false to disable
pidloop:Zone_Temp_0, and see how it affects
the zone final pwm, when pidloop: Zone_Temp_0
in zone 0 is disabled.
then even we are trying to heat up the temperature
of a sensor: Temp_0 in pidloop: Zone_Temp_0, this
set point of the pidloop will not be taken into the
calculation for the final set point of the whole zone.
```
<service object>
root@openbmc:/tmp# busctl tree xyz.openbmc_project.State.FanCtrl
`-/xyz
`-/xyz/openbmc_project
`-/xyz/openbmc_project/settings
`-/xyz/openbmc_project/settings/fanctrl
|-/xyz/openbmc_project/settings/fanctrl/zone0
| |-/xyz/openbmc_project/settings/fanctrl/zone0/Zone_Temp
| |-/xyz/openbmc_project/settings/fanctrl/zone0/Zone_Temp_0
| `-/xyz/openbmc_project/settings/fanctrl/zone0/Zone_Temp_1
====Enable process for pidloop:Zone_Temp_0 with p-switch temperature sensor:Temp_0 at runtime====
root@openbmc:~# busctl introspect xyz.openbmc_project.State.FanCtrl /xyz/openbmc_project/settings/fanctrl/zone0/Zone_Temp_0
NAME TYPE SIGNATURE RESULT/VALUE FLAGS
xyz.openbmc_project.Object.Enable interface - - -
.Enabled property b true emits-change writable
====Disable process for pidloop:Zone_Temp_0 with p-switch temperature sensor: Temp_0====
root@openbmc:~# busctl set-property xyz.openbmc_project.State.FanCtrl /xyz/openbmc_project/settings/fanctrl/zone0/Zone_Temp_0 xyz.openbmc_project.Object.Enable Enabled b false
root@openbmc:~# busctl introspect xyz.openbmc_project.State.FanCtrl /xyz/openbmc_project/settings/fanctrl/zone0/Zone_Temp_0
NAME TYPE SIGNATURE RESULT/VALUE FLAGS
xyz.openbmc_project.Object.Enable interface - - -
.Enabled property b false emits-change writable
```
when Disable the process of the pidloop: Zone_Temp_0,
the requester switches from Zone_Temp_0 to the others,
when you enable the pidloop: Zone_Temp_0, the setpoint
of Zone_Temp_0 will be take into consideration again
Change-Id: I95ae700144f0d16049fff8b309f05ae690a7ef72
Signed-off-by: ykchiu <Chiu.YK@inventec.com>
diff --git a/test/helpers.hpp b/test/helpers.hpp
index 8f81bc6..4d3faea 100644
--- a/test/helpers.hpp
+++ b/test/helpers.hpp
@@ -45,6 +45,7 @@
EXPECT_CALL(*sdbus_mock,
sd_bus_add_object_vtable(IsNull(), NotNull(), StrEq(path),
StrEq(intf), NotNull(), NotNull()))
+ .Times(::testing::AnyNumber())
.WillOnce(Return(0));
if (!defer)
diff --git a/test/pid_zone_unittest.cpp b/test/pid_zone_unittest.cpp
index 8e74367..4eb3487 100644
--- a/test/pid_zone_unittest.cpp
+++ b/test/pid_zone_unittest.cpp
@@ -26,6 +26,7 @@
using ::testing::StrEq;
static std::string modeInterface = "xyz.openbmc_project.Control.Mode";
+static std::string enableInterface = "xyz.openbmc_project.Object.Enable";
namespace
{
@@ -34,10 +35,12 @@
{
// Build a PID Zone.
- sdbusplus::SdBusMock sdbus_mock_passive, sdbus_mock_host, sdbus_mock_mode;
+ sdbusplus::SdBusMock sdbus_mock_passive, sdbus_mock_host, sdbus_mock_mode,
+ sdbus_mock_enable;
auto bus_mock_passive = sdbusplus::get_mocked_new(&sdbus_mock_passive);
auto bus_mock_host = sdbusplus::get_mocked_new(&sdbus_mock_host);
auto bus_mock_mode = sdbusplus::get_mocked_new(&sdbus_mock_mode);
+ auto bus_mock_enable = sdbusplus::get_mocked_new(&sdbus_mock_enable);
EXPECT_CALL(sdbus_mock_host,
sd_bus_add_object_manager(
@@ -58,6 +61,15 @@
SetupDbusObject(&sdbus_mock_mode, defer, objPath, modeInterface, properties,
&d);
+ std::string sensorname = "temp1";
+ std::string pidsensorpath = "/xyz/openbmc_project/settings/fanctrl/zone1/" +
+ sensorname;
+
+ double de;
+ std::vector<std::string> propertiesenable;
+ SetupDbusObject(&sdbus_mock_enable, defer, pidsensorpath.c_str(),
+ enableInterface, propertiesenable, &de);
+
DbusPidZone p(zone, minThermalOutput, failSafePercent, cycleTime, m,
bus_mock_mode, objPath, defer);
// Success.
@@ -70,7 +82,7 @@
protected:
PidZoneTest() :
property_index(), properties(), sdbus_mock_passive(), sdbus_mock_host(),
- sdbus_mock_mode()
+ sdbus_mock_mode(), sdbus_mock_enable()
{
EXPECT_CALL(sdbus_mock_host,
sd_bus_add_object_manager(
@@ -80,6 +92,7 @@
auto bus_mock_passive = sdbusplus::get_mocked_new(&sdbus_mock_passive);
auto bus_mock_host = sdbusplus::get_mocked_new(&sdbus_mock_host);
auto bus_mock_mode = sdbusplus::get_mocked_new(&sdbus_mock_mode);
+ auto bus_mock_enable = sdbusplus::get_mocked_new(&sdbus_mock_enable);
// Compiler weirdly not happy about just instantiating mgr(...);
SensorManager m(bus_mock_passive, bus_mock_host);
@@ -88,6 +101,10 @@
SetupDbusObject(&sdbus_mock_mode, defer, objPath, modeInterface,
properties, &property_index);
+ SetupDbusObject(&sdbus_mock_enable, defer, pidsensorpath.c_str(),
+ enableInterface, propertiesenable,
+ &propertyenable_index);
+
zone = std::make_unique<DbusPidZone>(zoneId, minThermalOutput,
failSafePercent, cycleTime, mgr,
bus_mock_mode, objPath, defer);
@@ -96,10 +113,13 @@
// unused
double property_index;
std::vector<std::string> properties;
+ double propertyenable_index;
+ std::vector<std::string> propertiesenable;
sdbusplus::SdBusMock sdbus_mock_passive;
sdbusplus::SdBusMock sdbus_mock_host;
sdbusplus::SdBusMock sdbus_mock_mode;
+ sdbusplus::SdBusMock sdbus_mock_enable;
int64_t zoneId = 1;
double minThermalOutput = 1000.0;
double failSafePercent = 0.75;
@@ -108,6 +128,10 @@
SensorManager mgr;
conf::CycleTime cycleTime;
+ std::string sensorname = "temp1";
+ std::string pidsensorpath = "/xyz/openbmc_project/settings/fanctrl/zone1/" +
+ sensorname;
+
std::unique_ptr<DbusPidZone> zone;
};
@@ -128,6 +152,28 @@
EXPECT_TRUE(zone->getManualMode());
}
+TEST_F(PidZoneTest, AddPidControlProcessGetAndSetEnableTest_BehavesAsExpected)
+{
+ // Verifies that the zone starts in enable mode. Verifies that one can set
+ // enable the mode.
+ auto bus_mock_enable = sdbusplus::get_mocked_new(&sdbus_mock_enable);
+
+ EXPECT_CALL(sdbus_mock_mode, sd_bus_emit_properties_changed_strv(
+ IsNull(), StrEq(pidsensorpath.c_str()),
+ StrEq(enableInterface), NotNull()))
+ .Times(::testing::AnyNumber())
+ .WillOnce(Invoke(
+ [&]([[maybe_unused]] sd_bus* bus, [[maybe_unused]] const char* path,
+ [[maybe_unused]] const char* interface, const char** names) {
+ EXPECT_STREQ("Enable", names[0]);
+ return 0;
+ }));
+
+ zone->addPidControlProcess(sensorname, bus_mock_enable,
+ pidsensorpath.c_str(), defer);
+ EXPECT_TRUE(zone->isPidProcessEnabled(sensorname));
+}
+
TEST_F(PidZoneTest, SetManualMode_RedundantWritesEnabledOnceAfterManualMode)
{
// Tests adding a fan PID controller to the zone, and verifies it's
@@ -166,12 +212,31 @@
// Tests addSetPoint, clearSetPoints, determineMaxSetPointRequest
// and getMinThermalSetPoint.
+ // Need to add pid control process for the zone that can enable
+ // the process and add the set point.
+ auto bus_mock_enable = sdbusplus::get_mocked_new(&sdbus_mock_enable);
+
+ EXPECT_CALL(sdbus_mock_mode, sd_bus_emit_properties_changed_strv(
+ IsNull(), StrEq(pidsensorpath.c_str()),
+ StrEq(enableInterface), NotNull()))
+ .Times(::testing::AnyNumber())
+ .WillOnce(Invoke(
+ [&]([[maybe_unused]] sd_bus* bus, [[maybe_unused]] const char* path,
+ [[maybe_unused]] const char* interface, const char** names) {
+ EXPECT_STREQ("Enable", names[0]);
+ return 0;
+ }));
+
+ zone->addPidControlProcess(sensorname, bus_mock_enable,
+ pidsensorpath.c_str(), defer);
+
// At least one value must be above the minimum thermal setpoint used in
// the constructor otherwise it'll choose that value
std::vector<double> values = {100, 200, 300, 400, 500, 5000};
+
for (auto v : values)
{
- zone->addSetPoint(v, "");
+ zone->addSetPoint(v, sensorname);
}
// This will pull the maximum RPM setpoint request.
@@ -191,10 +256,29 @@
// Tests adding several RPM setpoints, however, they're all lower than the
// configured minimal thermal setpoint RPM value.
+ // Need to add pid control process for the zone that can enable
+ // the process and add the set point.
+ auto bus_mock_enable = sdbusplus::get_mocked_new(&sdbus_mock_enable);
+
+ EXPECT_CALL(sdbus_mock_mode, sd_bus_emit_properties_changed_strv(
+ IsNull(), StrEq(pidsensorpath.c_str()),
+ StrEq(enableInterface), NotNull()))
+ .Times(::testing::AnyNumber())
+ .WillOnce(Invoke(
+ [&]([[maybe_unused]] sd_bus* bus, [[maybe_unused]] const char* path,
+ [[maybe_unused]] const char* interface, const char** names) {
+ EXPECT_STREQ("Enable", names[0]);
+ return 0;
+ }));
+
+ zone->addPidControlProcess(sensorname, bus_mock_enable,
+ pidsensorpath.c_str(), defer);
+
std::vector<double> values = {100, 200, 300, 400, 500};
+
for (auto v : values)
{
- zone->addSetPoint(v, "");
+ zone->addSetPoint(v, sensorname);
}
// This will pull the maximum RPM setpoint request.