controller: Heap-allocate LEDs
Do this in preparation for migrating to a single instance of the daemon
whose LED inventory is updated via DBus where the update is triggered by
udev.
Splitting out this work reduces the review complexity of the patch that
rewrites the internals of the daemon.
Signed-off-by: Jayashree Dhanapal <jayashree-d@hcl.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I50e58126413514f7a64153f425629142fa5af6d4
Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>
diff --git a/controller.cpp b/controller.cpp
index fd08c74..695f174 100644
--- a/controller.cpp
+++ b/controller.cpp
@@ -146,8 +146,8 @@
// Create the Physical LED objects for directing actions.
// Need to save this else sdbusplus destructor will wipe this off.
- phosphor::led::SysfsLed sled{fs::path(path)};
- phosphor::led::Physical led(bus, objPath, sled, ledDescr.color);
+ auto sled = std::make_unique<phosphor::led::SysfsLed>(fs::path(path));
+ phosphor::led::Physical led(bus, objPath, std::move(sled), ledDescr.color);
/** @brief Claim the bus */
bus.request_name(busName.c_str());
diff --git a/physical.cpp b/physical.cpp
index 67f0a90..39c1c07 100644
--- a/physical.cpp
+++ b/physical.cpp
@@ -28,13 +28,13 @@
/** @brief Populates key parameters */
void Physical::setInitialState()
{
- assert = led.getMaxBrightness();
- auto trigger = led.getTrigger();
+ assert = led->getMaxBrightness();
+ auto trigger = led->getTrigger();
if (trigger == "timer")
{
// LED is blinking. Get the on and off delays and derive percent duty
- auto delayOn = led.getDelayOn();
- uint16_t periodMs = delayOn + led.getDelayOff();
+ auto delayOn = led->getDelayOn();
+ uint16_t periodMs = delayOn + led->getDelayOff();
auto percentScale = periodMs / 100;
this->dutyOn(delayOn / percentScale);
this->period(periodMs);
@@ -44,7 +44,7 @@
else
{
// Cache current LED state
- auto brightness = led.getBrightness();
+ auto brightness = led->getBrightness();
if (brightness != 0U && assert != 0U)
{
sdbusplus::xyz::openbmc_project::Led::server::Physical::state(
@@ -96,8 +96,8 @@
{
auto value = (action == Action::On) ? assert : deasserted;
- led.setTrigger("none");
- led.setBrightness(value);
+ led->setTrigger("none");
+ led->setBrightness(value);
}
void Physical::blinkOperation()
@@ -120,9 +120,9 @@
auto p = static_cast<unsigned long>(period());
- led.setTrigger("timer");
- led.setDelayOn(p * d / 100UL);
- led.setDelayOff(p * (100UL - d) / 100UL);
+ led->setTrigger("timer");
+ led->setDelayOn(p * d / 100UL);
+ led->setDelayOff(p * (100UL - d) / 100UL);
}
/** @brief set led color property in DBus*/
diff --git a/physical.hpp b/physical.hpp
index 87bf5b5..14b7655 100644
--- a/physical.hpp
+++ b/physical.hpp
@@ -9,6 +9,8 @@
#include <fstream>
#include <string>
+namespace fs = std::filesystem;
+
namespace phosphor
{
namespace led
@@ -42,11 +44,13 @@
* @param[in] ledPath - sysfs path where this LED is exported
* @param[in] color - led color name
*/
- Physical(sdbusplus::bus_t& bus, const std::string& objPath, SysfsLed& led,
+
+ Physical(sdbusplus::bus_t& bus, const std::string& objPath,
+ std::unique_ptr<phosphor::led::SysfsLed> led,
const std::string& color = "") :
PhysicalIfaces(bus, objPath.c_str(),
PhysicalIfaces::action::defer_emit),
- led(led)
+ led(std::move(led))
{
// Suppose this is getting launched as part of BMC reboot, then we
// need to save what the micro-controller currently has.
@@ -75,7 +79,7 @@
private:
/** @brief Associated LED implementation
*/
- SysfsLed& led;
+ std::unique_ptr<phosphor::led::SysfsLed> led;
/** @brief The value that will assert the LED */
unsigned long assert{};
diff --git a/test/physical.cpp b/test/physical.cpp
index 271143c..90ced2d 100644
--- a/test/physical.cpp
+++ b/test/physical.cpp
@@ -76,9 +76,9 @@
{
sdbusplus::bus_t bus = sdbusplus::bus::new_default();
/* NiceMock ignores calls to methods with no expectations defined */
- NiceMock<MockLed> led;
- ON_CALL(led, getTrigger()).WillByDefault(Return("none"));
- phosphor::led::Physical phy(bus, ledObj, led);
+ auto led = std::make_unique<NiceMock<MockLed>>();
+ ON_CALL(*led, getTrigger()).WillByDefault(Return("none"));
+ phosphor::led::Physical phy(bus, ledObj, std::move(led));
EXPECT_EQ(phy.state(), Action::Off);
}
@@ -86,11 +86,11 @@
{
sdbusplus::bus_t bus = sdbusplus::bus::new_default();
/* NiceMock ignores calls to methods with no expectations defined */
- NiceMock<MockLed> led;
- EXPECT_CALL(led, getTrigger()).WillRepeatedly(Return("none"));
- EXPECT_CALL(led, getBrightness()).WillOnce(Return(127));
- EXPECT_CALL(led, getMaxBrightness()).WillOnce(Return(127));
- phosphor::led::Physical phy(bus, ledObj, led);
+ auto led = std::make_unique<NiceMock<MockLed>>();
+ EXPECT_CALL(*led, getTrigger()).WillRepeatedly(Return("none"));
+ EXPECT_CALL(*led, getBrightness()).WillOnce(Return(127));
+ EXPECT_CALL(*led, getMaxBrightness()).WillOnce(Return(127));
+ phosphor::led::Physical phy(bus, ledObj, std::move(led));
EXPECT_EQ(phy.state(), Action::On);
}
@@ -98,11 +98,11 @@
{
sdbusplus::bus_t bus = sdbusplus::bus::new_default();
/* NiceMock ignores calls to methods with no expectations defined */
- NiceMock<MockLed> led;
- EXPECT_CALL(led, getTrigger()).WillRepeatedly(Return("none"));
- EXPECT_CALL(led, getBrightness()).WillOnce(Return(0));
- EXPECT_CALL(led, getMaxBrightness()).WillOnce(Return(0));
- phosphor::led::Physical phy(bus, ledObj, led);
+ auto led = std::make_unique<NiceMock<MockLed>>();
+ EXPECT_CALL(*led, getTrigger()).WillRepeatedly(Return("none"));
+ EXPECT_CALL(*led, getBrightness()).WillOnce(Return(0));
+ EXPECT_CALL(*led, getMaxBrightness()).WillOnce(Return(0));
+ phosphor::led::Physical phy(bus, ledObj, std::move(led));
EXPECT_EQ(phy.state(), Action::Off);
}
@@ -110,11 +110,11 @@
{
sdbusplus::bus_t bus = sdbusplus::bus::new_default();
/* NiceMock ignores calls to methods with no expectations defined */
- NiceMock<MockLed> led;
- EXPECT_CALL(led, getTrigger()).WillRepeatedly(Return("none"));
- EXPECT_CALL(led, getBrightness()).WillOnce(Return(0));
- EXPECT_CALL(led, getMaxBrightness()).WillOnce(Return(127));
- phosphor::led::Physical phy(bus, ledObj, led);
+ auto led = std::make_unique<NiceMock<MockLed>>();
+ EXPECT_CALL(*led, getTrigger()).WillRepeatedly(Return("none"));
+ EXPECT_CALL(*led, getBrightness()).WillOnce(Return(0));
+ EXPECT_CALL(*led, getMaxBrightness()).WillOnce(Return(127));
+ phosphor::led::Physical phy(bus, ledObj, std::move(led));
EXPECT_EQ(phy.state(), Action::Off);
}
@@ -122,35 +122,35 @@
{
sdbusplus::bus_t bus = sdbusplus::bus::new_default();
/* NiceMock ignores calls to methods with no expectations defined */
- NiceMock<MockLed> led;
- EXPECT_CALL(led, getTrigger()).WillRepeatedly(Return("none"));
- EXPECT_CALL(led, getBrightness()).WillOnce(Return(127));
- EXPECT_CALL(led, getMaxBrightness()).WillOnce(Return(0));
- phosphor::led::Physical phy(bus, ledObj, led);
+ auto led = std::make_unique<NiceMock<MockLed>>();
+ EXPECT_CALL(*led, getTrigger()).WillRepeatedly(Return("none"));
+ EXPECT_CALL(*led, getBrightness()).WillOnce(Return(127));
+ EXPECT_CALL(*led, getMaxBrightness()).WillOnce(Return(0));
+ phosphor::led::Physical phy(bus, ledObj, std::move(led));
EXPECT_EQ(phy.state(), Action::Off);
}
TEST(Physical, ctor_timer_trigger)
{
sdbusplus::bus_t bus = sdbusplus::bus::new_default();
- NiceMock<MockLed> led;
- EXPECT_CALL(led, getTrigger()).WillOnce(Return("timer"));
- EXPECT_CALL(led, getDelayOn()).WillOnce(Return(500));
- EXPECT_CALL(led, getDelayOff()).WillOnce(Return(500));
- phosphor::led::Physical phy(bus, ledObj, led);
+ auto led = std::make_unique<NiceMock<MockLed>>();
+ EXPECT_CALL(*led, getTrigger()).WillOnce(Return("timer"));
+ EXPECT_CALL(*led, getDelayOn()).WillOnce(Return(500));
+ EXPECT_CALL(*led, getDelayOff()).WillOnce(Return(500));
+ phosphor::led::Physical phy(bus, ledObj, std::move(led));
EXPECT_EQ(phy.state(), Action::Blink);
}
TEST(Physical, off)
{
sdbusplus::bus_t bus = sdbusplus::bus::new_default();
- NiceMock<MockLed> led;
- ON_CALL(led, getMaxBrightness()).WillByDefault(Return(127));
- EXPECT_CALL(led, getTrigger()).WillOnce(Return("none"));
- EXPECT_CALL(led, getBrightness())
+ auto led = std::make_unique<NiceMock<MockLed>>();
+ ON_CALL(*led, getMaxBrightness()).WillByDefault(Return(127));
+ EXPECT_CALL(*led, getTrigger()).WillOnce(Return("none"));
+ EXPECT_CALL(*led, getBrightness())
.WillOnce(Return(phosphor::led::deasserted));
- EXPECT_CALL(led, setBrightness(phosphor::led::deasserted)).Times(0);
- phosphor::led::Physical phy(bus, ledObj, led);
+ EXPECT_CALL(*led, setBrightness(phosphor::led::deasserted)).Times(0);
+ phosphor::led::Physical phy(bus, ledObj, std::move(led));
phy.state(Action::Off);
EXPECT_EQ(phy.state(), Action::Off);
}
@@ -160,12 +160,12 @@
constexpr unsigned long asserted = 127;
sdbusplus::bus_t bus = sdbusplus::bus::new_default();
- NiceMock<MockLed> led;
- ON_CALL(led, getMaxBrightness()).WillByDefault(Return(asserted));
- EXPECT_CALL(led, getTrigger()).WillOnce(Return("none"));
- EXPECT_CALL(led, setTrigger("none"));
- EXPECT_CALL(led, setBrightness(asserted));
- phosphor::led::Physical phy(bus, ledObj, led);
+ auto led = std::make_unique<NiceMock<MockLed>>();
+ ON_CALL(*led, getMaxBrightness()).WillByDefault(Return(asserted));
+ EXPECT_CALL(*led, getTrigger()).WillOnce(Return("none"));
+ EXPECT_CALL(*led, setTrigger("none"));
+ EXPECT_CALL(*led, setBrightness(asserted));
+ phosphor::led::Physical phy(bus, ledObj, std::move(led));
phy.state(Action::On);
EXPECT_EQ(phy.state(), Action::On);
}
@@ -173,12 +173,12 @@
TEST(Physical, blink)
{
sdbusplus::bus_t bus = sdbusplus::bus::new_default();
- NiceMock<MockLed> led;
- EXPECT_CALL(led, getTrigger()).WillOnce(Return("none"));
- EXPECT_CALL(led, setTrigger("timer"));
- EXPECT_CALL(led, setDelayOn(500));
- EXPECT_CALL(led, setDelayOff(500));
- phosphor::led::Physical phy(bus, ledObj, led);
+ auto led = std::make_unique<NiceMock<MockLed>>();
+ EXPECT_CALL(*led, getTrigger()).WillOnce(Return("none"));
+ EXPECT_CALL(*led, setTrigger("timer"));
+ EXPECT_CALL(*led, setDelayOn(500));
+ EXPECT_CALL(*led, setDelayOff(500));
+ phosphor::led::Physical phy(bus, ledObj, std::move(led));
phy.state(Action::Blink);
EXPECT_EQ(phy.state(), Action::Blink);
}
@@ -186,10 +186,10 @@
TEST(Physical, ctor_none_trigger_asserted_brightness)
{
sdbusplus::bus_t bus = sdbusplus::bus::new_default();
- NiceMock<MockLed> led;
- EXPECT_CALL(led, getTrigger()).WillRepeatedly(Return("none"));
- EXPECT_CALL(led, getBrightness()).WillRepeatedly(Return(127));
- phosphor::led::Physical phy(bus, ledObj, led);
+ auto led = std::make_unique<NiceMock<MockLed>>();
+ EXPECT_CALL(*led, getTrigger()).WillRepeatedly(Return("none"));
+ EXPECT_CALL(*led, getBrightness()).WillRepeatedly(Return(127));
+ phosphor::led::Physical phy(bus, ledObj, std::move(led));
EXPECT_EQ(phy.state(), Action::Off);
}
@@ -199,14 +199,14 @@
constexpr unsigned long asserted = 127;
auto bus = sdbusplus::bus::new_default();
- NiceMock<MockLed> led;
- ON_CALL(led, getMaxBrightness()).WillByDefault(Return(asserted));
- EXPECT_CALL(led, getTrigger()).Times(1).WillOnce(Return("none"));
- constexpr auto deasserted = phosphor::led::deasserted;
- EXPECT_CALL(led, getBrightness()).WillOnce(Return(deasserted));
- EXPECT_CALL(led, setBrightness(asserted));
- EXPECT_CALL(led, setBrightness(deasserted));
- phosphor::led::Physical phy(bus, ledObj, led);
+ auto led = std::make_unique<NiceMock<MockLed>>();
+ ON_CALL(*led, getMaxBrightness()).WillByDefault(Return(asserted));
+ EXPECT_CALL(*led, getTrigger()).Times(1).WillOnce(Return("none"));
+ EXPECT_CALL(*led, getBrightness())
+ .WillOnce(Return(phosphor::led::deasserted));
+ EXPECT_CALL(*led, setBrightness(asserted));
+ EXPECT_CALL(*led, setBrightness(phosphor::led::deasserted));
+ phosphor::led::Physical phy(bus, ledObj, std::move(led));
phy.state(Action::On);
EXPECT_EQ(phy.state(), Action::On);
phy.state(Action::Off);