regulators: Complete PresenceDetection class
Complete implementation of the PresenceDetection class. Implement the
execute() method that executes one or more actions to determine if a
device is present.
Cache the results so that subsequent calls to execute() will return the
cached value. Executing the actions may be expensive, requiring one or
more D-Bus calls.
Provide a clearCache() method to be called on each boot to clear the
cached value.
Signed-off-by: Bob King <Bob_King@wistron.com>
Change-Id: Ie0e3dfde10f2df8ca8b56ec14ce723f356e97dfc
diff --git a/phosphor-regulators/src/meson.build b/phosphor-regulators/src/meson.build
index 8238d93..7b852bb 100644
--- a/phosphor-regulators/src/meson.build
+++ b/phosphor-regulators/src/meson.build
@@ -15,6 +15,7 @@
'id_map.cpp',
'journal.cpp',
'pmbus_utils.cpp',
+ 'presence_detection.cpp',
'presence_service.cpp',
'rail.cpp',
'sensor_monitoring.cpp',
diff --git a/phosphor-regulators/src/presence_detection.cpp b/phosphor-regulators/src/presence_detection.cpp
new file mode 100644
index 0000000..3ed5350
--- /dev/null
+++ b/phosphor-regulators/src/presence_detection.cpp
@@ -0,0 +1,64 @@
+/**
+ * Copyright © 2020 IBM Corporation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "presence_detection.hpp"
+
+#include "action_environment.hpp"
+#include "action_utils.hpp"
+#include "chassis.hpp"
+#include "device.hpp"
+#include "exception_utils.hpp"
+#include "system.hpp"
+
+#include <exception>
+
+namespace phosphor::power::regulators
+{
+
+bool PresenceDetection::execute(Services& services, System& system,
+ Chassis& /*chassis*/, Device& device)
+{
+ // If no presence value is cached
+ if (!isPresent.has_value())
+ {
+ // Initially assume device is present
+ isPresent = true;
+
+ // Execute actions to find device presence
+ try
+ {
+ // Create ActionEnvironment
+ ActionEnvironment environment{system.getIDMap(), device.getID(),
+ services};
+
+ // Execute the actions and cache resulting value
+ isPresent = action_utils::execute(actions, environment);
+ }
+ catch (const std::exception& e)
+ {
+ // Log error messages in journal
+ services.getJournal().logError(exception_utils::getMessages(e));
+ services.getJournal().logError("Unable to determine presence of " +
+ device.getID());
+
+ // TODO: Create error log entry
+ }
+ }
+
+ return isPresent.value();
+}
+
+} // namespace phosphor::power::regulators
diff --git a/phosphor-regulators/src/presence_detection.hpp b/phosphor-regulators/src/presence_detection.hpp
index 23f416c..0574003 100644
--- a/phosphor-regulators/src/presence_detection.hpp
+++ b/phosphor-regulators/src/presence_detection.hpp
@@ -16,14 +16,21 @@
#pragma once
#include "action.hpp"
+#include "services.hpp"
#include <memory>
+#include <optional>
#include <utility>
#include <vector>
namespace phosphor::power::regulators
{
+// Forward declarations to avoid circular dependencies
+class Chassis;
+class Device;
+class System;
+
/**
* @class PresenceDetection
*
@@ -69,20 +76,30 @@
}
/**
+ * Clears the cached presence value.
+ */
+ void clearCache(void)
+ {
+ isPresent.reset();
+ }
+
+ /**
* Executes the actions to detect whether the device is present.
*
* The return value of the last action indicates whether the device is
* present. A return value of true means the device is present; false means
* the device is missing.
*
+ * Caches the resulting presence value. Subsequent calls to execute() will
+ * return the cached value rather than re-executing the actions. This
+ * provides a performance improvement since the actions may be expensive to
+ * execute, such as I2C reads or D-Bus method calls. The cached value can
+ * be cleared by calling clearCache().
+ *
* @return true if device is present, false otherwise
*/
- bool execute()
- {
- // TODO: Create ActionEnvironment, execute actions, catch and handle any
- // exceptions
- return true;
- }
+ bool execute(Services& services, System& system, Chassis& chassis,
+ Device& device);
/**
* Returns the actions that detect whether the device is present.
@@ -94,11 +111,26 @@
return actions;
}
+ /**
+ * Returns the cached presence value, if any.
+ *
+ * @return cached presence value
+ */
+ std::optional<bool> getCachedPresence() const
+ {
+ return isPresent;
+ }
+
private:
/**
* Actions that detect whether the device is present.
*/
std::vector<std::unique_ptr<Action>> actions{};
+
+ /**
+ * Cached presence value. Initially has no value.
+ */
+ std::optional<bool> isPresent{};
};
} // namespace phosphor::power::regulators
diff --git a/phosphor-regulators/test/presence_detection_tests.cpp b/phosphor-regulators/test/presence_detection_tests.cpp
index 7f3c10f..f489b7f 100644
--- a/phosphor-regulators/test/presence_detection_tests.cpp
+++ b/phosphor-regulators/test/presence_detection_tests.cpp
@@ -14,29 +14,252 @@
* limitations under the License.
*/
#include "action.hpp"
+#include "chassis.hpp"
+#include "compare_presence_action.hpp"
+#include "device.hpp"
+#include "i2c_interface.hpp"
#include "mock_action.hpp"
+#include "mock_journal.hpp"
+#include "mock_presence_service.hpp"
+#include "mock_services.hpp"
+#include "mocked_i2c_interface.hpp"
#include "presence_detection.hpp"
+#include "rule.hpp"
+#include "system.hpp"
#include <memory>
+#include <stdexcept>
+#include <string>
+#include <tuple>
#include <utility>
#include <vector>
+#include <gmock/gmock.h>
#include <gtest/gtest.h>
using namespace phosphor::power::regulators;
+using ::testing::Return;
+using ::testing::Throw;
+
+/**
+ * Creates the parent objects that normally contain a PresenceDetection object.
+ *
+ * A PresenceDetection object is normally contained within a hierarchy of
+ * System, Chassis, and Device objects. These objects are required in order to
+ * call the execute() method.
+ *
+ * Creates the System, Chassis, and Device objects. The PresenceDetection
+ * object is moved into the Device object.
+ *
+ * @param detection PresenceDetection object to move into object hierarchy
+ * @return Pointers to the System, Chassis, and Device objects. The Chassis and
+ * Device objects are contained within the System object and will be
+ * automatically destructed.
+ */
+std::tuple<std::unique_ptr<System>, Chassis*, Device*>
+ createParentObjects(std::unique_ptr<PresenceDetection> detection)
+{
+ // Create mock I2CInterface
+ std::unique_ptr<i2c::MockedI2CInterface> i2cInterface =
+ std::make_unique<i2c::MockedI2CInterface>();
+
+ // Create Device that contains PresenceDetection
+ std::unique_ptr<Device> device = std::make_unique<Device>(
+ "vdd_reg", true,
+ "/xyz/openbmc_project/inventory/system/chassis/motherboard/reg2",
+ std::move(i2cInterface), std::move(detection));
+ Device* devicePtr = device.get();
+
+ // Create Chassis that contains Device
+ std::vector<std::unique_ptr<Device>> devices{};
+ devices.emplace_back(std::move(device));
+ std::unique_ptr<Chassis> chassis =
+ std::make_unique<Chassis>(1, std::move(devices));
+ Chassis* chassisPtr = chassis.get();
+
+ // Create System that contains Chassis
+ std::vector<std::unique_ptr<Rule>> rules{};
+ std::vector<std::unique_ptr<Chassis>> chassisVec{};
+ chassisVec.emplace_back(std::move(chassis));
+ std::unique_ptr<System> system =
+ std::make_unique<System>(std::move(rules), std::move(chassisVec));
+
+ return std::make_tuple(std::move(system), chassisPtr, devicePtr);
+}
+
TEST(PresenceDetectionTests, Constructor)
{
std::vector<std::unique_ptr<Action>> actions{};
- actions.push_back(std::make_unique<MockAction>());
+ actions.emplace_back(std::make_unique<MockAction>());
- PresenceDetection presenceDetection(std::move(actions));
- EXPECT_EQ(presenceDetection.getActions().size(), 1);
+ PresenceDetection detection{std::move(actions)};
+ EXPECT_EQ(detection.getActions().size(), 1);
+ EXPECT_FALSE(detection.getCachedPresence().has_value());
+}
+
+TEST(PresenceDetectionTests, ClearCache)
+{
+ // Create MockAction that will return true once
+ std::unique_ptr<MockAction> action = std::make_unique<MockAction>();
+ EXPECT_CALL(*action, execute).Times(1).WillOnce(Return(true));
+
+ // Create PresenceDetection
+ std::vector<std::unique_ptr<Action>> actions{};
+ actions.emplace_back(std::move(action));
+ PresenceDetection* detection = new PresenceDetection(std::move(actions));
+
+ // Create parent System, Chassis, and Device objects
+ auto [system, chassis, device] =
+ createParentObjects(std::unique_ptr<PresenceDetection>{detection});
+
+ // Verify that initially no presence value is cached
+ EXPECT_FALSE(detection->getCachedPresence().has_value());
+
+ // Call execute() which should obtain and cache presence value
+ MockServices services{};
+ EXPECT_TRUE(detection->execute(services, *system, *chassis, *device));
+
+ // Verify true presence value was cached
+ EXPECT_TRUE(detection->getCachedPresence().has_value());
+ EXPECT_TRUE(detection->getCachedPresence().value());
+
+ // Clear cached presence value
+ detection->clearCache();
+
+ // Verify that no presence value is cached
+ EXPECT_FALSE(detection->getCachedPresence().has_value());
}
TEST(PresenceDetectionTests, Execute)
{
- // TODO: Implement test when execute() function is done
+ // Create ComparePresenceAction
+ std::unique_ptr<ComparePresenceAction> action =
+ std::make_unique<ComparePresenceAction>(
+ "/xyz/openbmc_project/inventory/system/chassis/motherboard/cpu2",
+ true);
+
+ // Create PresenceDetection
+ std::vector<std::unique_ptr<Action>> actions{};
+ actions.emplace_back(std::move(action));
+ PresenceDetection* detection = new PresenceDetection(std::move(actions));
+
+ // Create parent System, Chassis, and Device objects
+ auto [system, chassis, device] =
+ createParentObjects(std::unique_ptr<PresenceDetection>{detection});
+
+ // Test where works: Present: Value is not cached
+ {
+ EXPECT_FALSE(detection->getCachedPresence().has_value());
+
+ // Create MockServices. MockPresenceService::isPresent() should return
+ // true.
+ MockServices services{};
+ MockPresenceService& presenceService =
+ services.getMockPresenceService();
+ EXPECT_CALL(presenceService,
+ isPresent("/xyz/openbmc_project/inventory/system/chassis/"
+ "motherboard/cpu2"))
+ .Times(1)
+ .WillOnce(Return(true));
+
+ // Execute PresenceDetection
+ EXPECT_TRUE(detection->execute(services, *system, *chassis, *device));
+
+ EXPECT_TRUE(detection->getCachedPresence().has_value());
+ EXPECT_TRUE(detection->getCachedPresence().value());
+ }
+
+ // Test where works: Present: Value is cached
+ {
+ EXPECT_TRUE(detection->getCachedPresence().has_value());
+
+ // Create MockServices. MockPresenceService::isPresent() should not be
+ // called.
+ MockServices services{};
+ MockPresenceService& presenceService =
+ services.getMockPresenceService();
+ EXPECT_CALL(presenceService, isPresent).Times(0);
+
+ // Execute PresenceDetection
+ EXPECT_TRUE(detection->execute(services, *system, *chassis, *device));
+ }
+
+ // Test where works: Not present: Value is not cached
+ {
+ // Clear cached presence value
+ detection->clearCache();
+ EXPECT_FALSE(detection->getCachedPresence().has_value());
+
+ // Create MockServices. MockPresenceService::isPresent() should return
+ // false.
+ MockServices services{};
+ MockPresenceService& presenceService =
+ services.getMockPresenceService();
+ EXPECT_CALL(presenceService,
+ isPresent("/xyz/openbmc_project/inventory/system/chassis/"
+ "motherboard/cpu2"))
+ .Times(1)
+ .WillOnce(Return(false));
+
+ // Execute PresenceDetection
+ EXPECT_FALSE(detection->execute(services, *system, *chassis, *device));
+
+ EXPECT_TRUE(detection->getCachedPresence().has_value());
+ EXPECT_FALSE(detection->getCachedPresence().value());
+ }
+
+ // Test where works: Not present: Value is cached
+ {
+ EXPECT_TRUE(detection->getCachedPresence().has_value());
+
+ // Create MockServices. MockPresenceService::isPresent() should not be
+ // called.
+ MockServices services{};
+ MockPresenceService& presenceService =
+ services.getMockPresenceService();
+ EXPECT_CALL(presenceService, isPresent).Times(0);
+
+ // Execute PresenceDetection
+ EXPECT_FALSE(detection->execute(services, *system, *chassis, *device));
+ }
+
+ // Test where fails
+ {
+ // Clear cached presence value
+ detection->clearCache();
+ EXPECT_FALSE(detection->getCachedPresence().has_value());
+
+ // Create MockServices. MockPresenceService::isPresent() should throw
+ // an exception.
+ MockServices services{};
+ MockPresenceService& presenceService =
+ services.getMockPresenceService();
+ EXPECT_CALL(presenceService,
+ isPresent("/xyz/openbmc_project/inventory/system/chassis/"
+ "motherboard/cpu2"))
+ .Times(1)
+ .WillOnce(
+ Throw(std::runtime_error{"DBusError: Invalid object path."}));
+
+ // Define expected journal messages that should be passed to MockJournal
+ MockJournal& journal = services.getMockJournal();
+ std::vector<std::string> exceptionMessages{
+ "DBusError: Invalid object path.",
+ "ActionError: compare_presence: { fru: "
+ "/xyz/openbmc_project/inventory/system/chassis/motherboard/cpu2, "
+ "value: true }"};
+ EXPECT_CALL(journal, logError(exceptionMessages)).Times(1);
+ EXPECT_CALL(journal,
+ logError("Unable to determine presence of vdd_reg"))
+ .Times(1);
+
+ // Execute PresenceDetection. Should return true when an error occurs.
+ EXPECT_TRUE(detection->execute(services, *system, *chassis, *device));
+
+ EXPECT_TRUE(detection->getCachedPresence().has_value());
+ EXPECT_TRUE(detection->getCachedPresence().value());
+ }
}
TEST(PresenceDetectionTests, GetActions)
@@ -44,13 +267,46 @@
std::vector<std::unique_ptr<Action>> actions{};
MockAction* action1 = new MockAction{};
- actions.push_back(std::unique_ptr<MockAction>{action1});
+ actions.emplace_back(std::unique_ptr<MockAction>{action1});
MockAction* action2 = new MockAction{};
- actions.push_back(std::unique_ptr<MockAction>{action2});
+ actions.emplace_back(std::unique_ptr<MockAction>{action2});
- PresenceDetection presenceDetection(std::move(actions));
- EXPECT_EQ(presenceDetection.getActions().size(), 2);
- EXPECT_EQ(presenceDetection.getActions()[0].get(), action1);
- EXPECT_EQ(presenceDetection.getActions()[1].get(), action2);
+ PresenceDetection detection{std::move(actions)};
+ EXPECT_EQ(detection.getActions().size(), 2);
+ EXPECT_EQ(detection.getActions()[0].get(), action1);
+ EXPECT_EQ(detection.getActions()[1].get(), action2);
+}
+
+TEST(PresenceDetectionTests, GetCachedPresence)
+{
+ // Create MockAction that will return false once
+ std::unique_ptr<MockAction> action = std::make_unique<MockAction>();
+ EXPECT_CALL(*action, execute).Times(1).WillOnce(Return(false));
+
+ // Create PresenceDetection
+ std::vector<std::unique_ptr<Action>> actions{};
+ actions.emplace_back(std::move(action));
+ PresenceDetection* detection = new PresenceDetection(std::move(actions));
+
+ // Create parent System, Chassis, and Device objects
+ auto [system, chassis, device] =
+ createParentObjects(std::unique_ptr<PresenceDetection>{detection});
+
+ // Verify that initially no presence value is cached
+ EXPECT_FALSE(detection->getCachedPresence().has_value());
+
+ // Call execute() which should obtain and cache presence value
+ MockServices services{};
+ EXPECT_FALSE(detection->execute(services, *system, *chassis, *device));
+
+ // Verify false presence value was cached
+ EXPECT_TRUE(detection->getCachedPresence().has_value());
+ EXPECT_FALSE(detection->getCachedPresence().value());
+
+ // Clear cached presence value
+ detection->clearCache();
+
+ // Verify that no presence value is cached
+ EXPECT_FALSE(detection->getCachedPresence().has_value());
}