Add IpmbSensor unit tests
Because everything was packed into IpmbSensor.cpp, which also included
main(), this commit breaks it up into the patterns we have in other
sensors, with <X>main.cpp containing the things for main.
While this doesn't clean up everything, it at least makes
processResponse unit testable, and adds one minor test for it.
Change-Id: I1eabf650ff635211e5b9a94816c2ffce250ed252
Signed-off-by: Ed Tanous <ed@tanous.net>
diff --git a/src/IpmbSensor.cpp b/src/IpmbSensor.cpp
index 5b4649b..30531bf 100644
--- a/src/IpmbSensor.cpp
+++ b/src/IpmbSensor.cpp
@@ -25,13 +25,10 @@
#include <boost/asio/error.hpp>
#include <boost/asio/io_context.hpp>
-#include <boost/asio/post.hpp>
#include <boost/asio/steady_timer.hpp>
#include <boost/container/flat_map.hpp>
#include <sdbusplus/asio/connection.hpp>
#include <sdbusplus/asio/object_server.hpp>
-#include <sdbusplus/bus.hpp>
-#include <sdbusplus/bus/match.hpp>
#include <sdbusplus/message.hpp>
#include <sdbusplus/message/native_types.hpp>
@@ -40,7 +37,6 @@
#include <chrono>
#include <cstddef>
#include <cstdint>
-#include <functional>
#include <iostream>
#include <limits>
#include <memory>
@@ -53,9 +49,6 @@
constexpr const bool debug = false;
-constexpr const char* sensorType = "IpmbSensor";
-constexpr const char* sdrInterface = "IpmbDevice";
-
static constexpr double ipmbMaxReading = 0xFF;
static constexpr double ipmbMinReading = 0;
@@ -67,11 +60,6 @@
static constexpr const char* sensorPathPrefix = "/xyz/openbmc_project/sensors/";
-boost::container::flat_map<std::string, std::shared_ptr<IpmbSensor>> sensors;
-boost::container::flat_map<uint8_t, std::shared_ptr<IpmbSDRDevice>> sdrsensor;
-
-std::unique_ptr<boost::asio::steady_timer> initCmdTimer;
-
IpmbSensor::IpmbSensor(std::shared_ptr<sdbusplus::asio::connection>& conn,
boost::asio::io_context& io,
const std::string& sensorName,
@@ -279,7 +267,9 @@
thresholds::checkThresholds(this);
}
-bool IpmbSensor::processReading(const std::vector<uint8_t>& data, double& resp)
+bool IpmbSensor::processReading(ReadingFormat readingFormat, uint8_t command,
+ const std::vector<uint8_t>& data, double& resp,
+ size_t errCount)
{
switch (readingFormat)
{
@@ -299,8 +289,7 @@
{
if (errCount == 0U)
{
- std::cerr << "Invalid data length returned for " << name
- << "\n";
+ std::cerr << "Invalid data length returned\n";
}
return false;
}
@@ -313,8 +302,7 @@
{
if (errCount == 0U)
{
- std::cerr << "Invalid data length returned for " << name
- << "\n";
+ std::cerr << "Invalid data length returned\n";
}
return false;
}
@@ -329,8 +317,7 @@
{
if (errCount == 0U)
{
- std::cerr << "Invalid data length returned for " << name
- << "\n";
+ std::cerr << "Invalid data length returned\n";
}
return false;
}
@@ -344,8 +331,7 @@
{
if (errCount == 0U)
{
- std::cerr << "Invalid data length returned for " << name
- << "\n";
+ std::cerr << "Invalid data length returned\n";
}
return false;
}
@@ -391,7 +377,7 @@
double value = 0;
- if (!processReading(data, value))
+ if (!processReading(readingFormat, command, data, value, errCount))
{
incrementError();
read();
@@ -558,8 +544,7 @@
std::vector<thresholds::Threshold> sensorThresholds;
if (!parseThresholdsFromConfig(interfaces, sensorThresholds))
{
- std::cerr << "error populating thresholds for " << name
- << "\n";
+ std::cerr << "error populating thresholds " << name << "\n";
}
uint8_t deviceAddress = loadVariant<uint8_t>(cfg, "Address");
@@ -616,69 +601,6 @@
"org.freedesktop.DBus.ObjectManager", "GetManagedObjects");
}
-void sdrHandler(sdbusplus::message_t& message,
- std::shared_ptr<sdbusplus::asio::connection>& dbusConnection)
-{
- std::string objectName;
- SensorBaseConfigMap values;
- message.read(objectName, values);
-
- auto findBus = values.find("Bus");
- if (findBus == values.end())
- {
- return;
- }
-
- uint8_t busIndex = loadVariant<uint8_t>(values, "Bus");
-
- auto& sdrsen = sdrsensor[busIndex];
- sdrsen = nullptr;
- sdrsen = std::make_shared<IpmbSDRDevice>(dbusConnection, busIndex);
- sdrsen->getSDRRepositoryInfo();
-}
-
-void reinitSensors(sdbusplus::message_t& message)
-{
- constexpr const size_t reinitWaitSeconds = 2;
- std::string objectName;
- boost::container::flat_map<std::string, std::variant<std::string>> values;
- message.read(objectName, values);
-
- auto findStatus = values.find(power::property);
- if (findStatus != values.end())
- {
- bool powerStatus =
- std::get<std::string>(findStatus->second).ends_with(".Running");
- if (powerStatus)
- {
- if (!initCmdTimer)
- {
- // this should be impossible
- return;
- }
- // we seem to send this command too fast sometimes, wait before
- // sending
- initCmdTimer->expires_after(
- std::chrono::seconds(reinitWaitSeconds));
-
- initCmdTimer->async_wait([](const boost::system::error_code ec) {
- if (ec == boost::asio::error::operation_aborted)
- {
- return; // we're being canceled
- }
-
- for (const auto& [name, sensor] : sensors)
- {
- if (sensor)
- {
- sensor->runInitCmd();
- }
- }
- });
- }
- }
-}
-
void interfaceRemoved(
sdbusplus::message_t& message,
boost::container::flat_map<std::string, std::shared_ptr<IpmbSensor>>&
@@ -712,68 +634,3 @@
}
}
}
-
-int main()
-{
- boost::asio::io_context io;
- auto systemBus = std::make_shared<sdbusplus::asio::connection>(io);
- sdbusplus::asio::object_server objectServer(systemBus, true);
- objectServer.add_manager("/xyz/openbmc_project/sensors");
- systemBus->request_name("xyz.openbmc_project.IpmbSensor");
-
- initCmdTimer = std::make_unique<boost::asio::steady_timer>(io);
-
- boost::asio::post(
- io, [&]() { createSensors(io, objectServer, sensors, systemBus); });
-
- boost::asio::steady_timer configTimer(io);
-
- std::function<void(sdbusplus::message_t&)> eventHandler =
- [&](sdbusplus::message_t&) {
- configTimer.expires_after(std::chrono::seconds(1));
- // create a timer because normally multiple properties change
- configTimer.async_wait([&](const boost::system::error_code& ec) {
- if (ec == boost::asio::error::operation_aborted)
- {
- return; // we're being canceled
- }
- createSensors(io, objectServer, sensors, systemBus);
- if (sensors.empty())
- {
- std::cout << "Configuration not detected\n";
- }
- });
- };
-
- std::vector<std::unique_ptr<sdbusplus::bus::match_t>> matches =
- setupPropertiesChangedMatches(
- *systemBus, std::to_array<const char*>({sensorType}), eventHandler);
-
- sdbusplus::bus::match_t powerChangeMatch(
- static_cast<sdbusplus::bus_t&>(*systemBus),
- "type='signal',interface='" + std::string(properties::interface) +
- "',path='" + std::string(power::path) + "',arg0='" +
- std::string(power::interface) + "'",
- reinitSensors);
-
- auto matchSignal = std::make_shared<sdbusplus::bus::match_t>(
- static_cast<sdbusplus::bus_t&>(*systemBus),
- "type='signal',member='PropertiesChanged',path_namespace='" +
- std::string(inventoryPath) + "',arg0namespace='" +
- configInterfaceName(sdrInterface) + "'",
- [&systemBus](sdbusplus::message_t& msg) {
- sdrHandler(msg, systemBus);
- });
-
- // Watch for entity-manager to remove configuration interfaces
- // so the corresponding sensors can be removed.
- auto ifaceRemovedMatch = std::make_shared<sdbusplus::bus::match_t>(
- static_cast<sdbusplus::bus_t&>(*systemBus),
- "type='signal',member='InterfacesRemoved',arg0path='" +
- std::string(inventoryPath) + "/'",
- [](sdbusplus::message_t& msg) { interfaceRemoved(msg, sensors); });
-
- setupManufacturingModeMatch(*systemBus);
- io.run();
- return 0;
-}
diff --git a/src/IpmbSensor.hpp b/src/IpmbSensor.hpp
index 58f20d5..d22e36d 100644
--- a/src/IpmbSensor.hpp
+++ b/src/IpmbSensor.hpp
@@ -10,6 +10,9 @@
#include <string>
#include <vector>
+constexpr const char* sensorType = "IpmbSensor";
+constexpr const char* sdrInterface = "IpmbDevice";
+
enum class IpmbType
{
none,
@@ -97,7 +100,9 @@
std::string getSubTypeUnits() const;
void loadDefaults();
void runInitCmd();
- bool processReading(const std::vector<uint8_t>& data, double& resp);
+ static bool processReading(ReadingFormat readingFormat, uint8_t command,
+ const std::vector<uint8_t>& data, double& resp,
+ size_t errCount);
void parseConfigValues(const SensorBaseConfigMap& entry);
bool sensorClassType(const std::string& sensorClass);
void sensorSubType(const std::string& sensorTypeName);
@@ -126,3 +131,14 @@
void ipmbRequestCompletionCb(const boost::system::error_code& ec,
const IpmbMethodType& response);
};
+
+void createSensors(
+ boost::asio::io_context& io, sdbusplus::asio::object_server& objectServer,
+ boost::container::flat_map<std::string, std::shared_ptr<IpmbSensor>>&
+ sensors,
+ std::shared_ptr<sdbusplus::asio::connection>& dbusConnection);
+
+void interfaceRemoved(
+ sdbusplus::message_t& message,
+ boost::container::flat_map<std::string, std::shared_ptr<IpmbSensor>>&
+ sensors);
diff --git a/src/IpmbSensorMain.cpp b/src/IpmbSensorMain.cpp
new file mode 100644
index 0000000..9223264
--- /dev/null
+++ b/src/IpmbSensorMain.cpp
@@ -0,0 +1,160 @@
+#include "IpmbSDRSensor.hpp"
+#include "IpmbSensor.hpp"
+#include "Utils.hpp"
+
+#include <boost/asio/error.hpp>
+#include <boost/asio/io_context.hpp>
+#include <boost/asio/post.hpp>
+#include <boost/asio/steady_timer.hpp>
+#include <boost/container/flat_map.hpp>
+#include <sdbusplus/asio/connection.hpp>
+#include <sdbusplus/asio/object_server.hpp>
+#include <sdbusplus/bus.hpp>
+#include <sdbusplus/bus/match.hpp>
+#include <sdbusplus/message.hpp>
+
+#include <array>
+#include <chrono>
+#include <cstddef>
+#include <cstdint>
+#include <functional>
+#include <iostream>
+#include <memory>
+#include <string>
+#include <variant>
+#include <vector>
+
+std::unique_ptr<boost::asio::steady_timer> initCmdTimer;
+boost::container::flat_map<std::string, std::shared_ptr<IpmbSensor>> sensors;
+boost::container::flat_map<uint8_t, std::shared_ptr<IpmbSDRDevice>> sdrsensor;
+
+void sdrHandler(
+ boost::container::flat_map<uint8_t, std::shared_ptr<IpmbSDRDevice>>
+ sdrsensor,
+ sdbusplus::message_t& message,
+ std::shared_ptr<sdbusplus::asio::connection>& dbusConnection)
+{
+ std::string objectName;
+ SensorBaseConfigMap values;
+ message.read(objectName, values);
+
+ auto findBus = values.find("Bus");
+ if (findBus == values.end())
+ {
+ return;
+ }
+
+ uint8_t busIndex = loadVariant<uint8_t>(values, "Bus");
+
+ auto& sdrsen = sdrsensor[busIndex];
+ sdrsen = nullptr;
+ sdrsen = std::make_shared<IpmbSDRDevice>(dbusConnection, busIndex);
+ sdrsen->getSDRRepositoryInfo();
+}
+
+void reinitSensors(sdbusplus::message_t& message)
+{
+ constexpr const size_t reinitWaitSeconds = 2;
+ std::string objectName;
+ boost::container::flat_map<std::string, std::variant<std::string>> values;
+ message.read(objectName, values);
+
+ auto findStatus = values.find(power::property);
+ if (findStatus != values.end())
+ {
+ bool powerStatus =
+ std::get<std::string>(findStatus->second).ends_with(".Running");
+ if (powerStatus)
+ {
+ if (!initCmdTimer)
+ {
+ // this should be impossible
+ return;
+ }
+ // we seem to send this command too fast sometimes, wait before
+ // sending
+ initCmdTimer->expires_after(
+ std::chrono::seconds(reinitWaitSeconds));
+
+ initCmdTimer->async_wait([](const boost::system::error_code ec) {
+ if (ec == boost::asio::error::operation_aborted)
+ {
+ return; // we're being canceled
+ }
+
+ for (const auto& [name, sensor] : sensors)
+ {
+ if (sensor)
+ {
+ sensor->runInitCmd();
+ }
+ }
+ });
+ }
+ }
+}
+
+int main()
+{
+ boost::asio::io_context io;
+ auto systemBus = std::make_shared<sdbusplus::asio::connection>(io);
+ sdbusplus::asio::object_server objectServer(systemBus, true);
+ objectServer.add_manager("/xyz/openbmc_project/sensors");
+ systemBus->request_name("xyz.openbmc_project.IpmbSensor");
+
+ initCmdTimer = std::make_unique<boost::asio::steady_timer>(io);
+
+ boost::asio::post(
+ io, [&]() { createSensors(io, objectServer, sensors, systemBus); });
+
+ boost::asio::steady_timer configTimer(io);
+
+ std::function<void(sdbusplus::message_t&)> eventHandler =
+ [&](sdbusplus::message_t&) {
+ configTimer.expires_after(std::chrono::seconds(1));
+ // create a timer because normally multiple properties change
+ configTimer.async_wait([&](const boost::system::error_code& ec) {
+ if (ec == boost::asio::error::operation_aborted)
+ {
+ return; // we're being canceled
+ }
+ createSensors(io, objectServer, sensors, systemBus);
+ if (sensors.empty())
+ {
+ std::cout << "Configuration not detected\n";
+ }
+ });
+ };
+
+ std::vector<std::unique_ptr<sdbusplus::bus::match_t>> matches =
+ setupPropertiesChangedMatches(
+ *systemBus, std::to_array<const char*>({sensorType}), eventHandler);
+
+ sdbusplus::bus::match_t powerChangeMatch(
+ static_cast<sdbusplus::bus_t&>(*systemBus),
+ "type='signal',interface='" + std::string(properties::interface) +
+ "',path='" + std::string(power::path) + "',arg0='" +
+ std::string(power::interface) + "'",
+ reinitSensors);
+
+ auto matchSignal = std::make_shared<sdbusplus::bus::match_t>(
+ static_cast<sdbusplus::bus_t&>(*systemBus),
+ "type='signal',member='PropertiesChanged',path_namespace='" +
+ std::string(inventoryPath) + "',arg0namespace='" +
+ configInterfaceName(sdrInterface) + "'",
+ [&systemBus](sdbusplus::message_t& msg) {
+ sdrHandler(sdrsensor, msg, systemBus);
+ });
+
+ // Watch for entity-manager to remove configuration interfaces
+ // so the corresponding sensors can be removed.
+ auto ifaceRemovedMatch = std::make_shared<sdbusplus::bus::match_t>(
+ static_cast<sdbusplus::bus_t&>(*systemBus),
+ "type='signal',member='InterfacesRemoved',arg0path='" +
+ std::string(inventoryPath) + "/'",
+ [](sdbusplus::message_t& msg) { interfaceRemoved(msg, sensors); });
+
+ setupManufacturingModeMatch(*systemBus);
+ io.run();
+ return 0;
+}
diff --git a/src/meson.build b/src/meson.build
index 429f7c5..6fb66a1 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -166,6 +166,7 @@
if get_option('ipmb').allowed()
executable(
'ipmbsensor',
+ 'IpmbSensorMain.cpp',
'IpmbSensor.cpp',
'IpmbSDRSensor.cpp',
dependencies: [
diff --git a/tests/meson.build b/tests/meson.build
index e5ad2ea..21f44a0 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -36,3 +36,22 @@
include_directories: '../src',
),
)
+
+test(
+ 'test_ipmb',
+ executable(
+ 'test_ipmb',
+ '../src/IpmbSensor.cpp',
+ '../src/Utils.cpp',
+ '../src/IpmbSDRSensor.cpp',
+ 'test_IpmbSensor.cpp',
+ dependencies: ut_deps_list,
+ link_with: [
+ utils_a,
+ thresholds_a,
+ devicemgmt_a
+ ],
+ implicit_include_directories: false,
+ include_directories: '../src',
+ ),
+)
diff --git a/tests/test_IpmbSensor.cpp b/tests/test_IpmbSensor.cpp
new file mode 100644
index 0000000..3168993
--- /dev/null
+++ b/tests/test_IpmbSensor.cpp
@@ -0,0 +1,23 @@
+#include "IpmbSensor.hpp"
+
+#include <cstddef>
+#include <cstdint>
+#include <vector>
+
+#include <gtest/gtest.h>
+
+namespace
+{
+
+TEST(IPMBSensor, Byte0)
+{
+ std::vector<uint8_t> data;
+ data.push_back(42);
+
+ double responseValue = 0.0;
+ size_t errCount = 0;
+ EXPECT_TRUE(IpmbSensor::processReading(ReadingFormat::byte0, 0, data,
+ responseValue, errCount));
+ EXPECT_EQ(responseValue, 42.0);
+}
+} // namespace