dbus-sensors: gpiosensor service: Add dbus retries
When the gpiosensor service starts, it makes a dbus call to get all the
dbus objects managed by Entity Manager. This dbus call sometimes fails,
and as a result, the gpiosensor service doesn't create dbus objects for
any GPIOs.
This commit updates the gpiosensor service to add dbus retries, if
needed.
Tested:
Ran the misseated_connection test 1000 times. Failure rate 1%.
http://sponge2/cb9b071d-3adb-42a4-8f54-3da18ff8d3d7
https://fusion2.corp.google.com/7e37c674-f1fd-3237-b8c0-d7d5f0ef9a28
https://fusion2.corp.google.com/89047ff5-446a-3525-8b42-e991e8dc7619
https://fusion2.corp.google.com/84d2983a-a18b-3de8-a616-84c1dc39b625
https://fusion2.corp.google.com/80a23722-ec58-33d2-bd5b-de685ce093f6
https://fusion2.corp.google.com/3e4c4b89-b96c-3a71-8e32-9a20a39eb809
Google-Bug-Id: 422474823
Platforms-Affected: All
Change-Id: I55abf9299ebe67d24ff2ce7bf85206a3f724f566
Signed-off-by: John Wedig <johnwedig@google.com>
(cherry picked from commit 029a4995388f1a7a5dfdf38fb3ca54ce7e0ec1dd)
diff --git a/recipes-phosphor/sensors/dbus-sensors/0003-gpiosensor-a-dedicated-daemon-to-report-gpio-status-.patch b/recipes-phosphor/sensors/dbus-sensors/0003-gpiosensor-a-dedicated-daemon-to-report-gpio-status-.patch
index aa6c60d..447fd0b 100644
--- a/recipes-phosphor/sensors/dbus-sensors/0003-gpiosensor-a-dedicated-daemon-to-report-gpio-status-.patch
+++ b/recipes-phosphor/sensors/dbus-sensors/0003-gpiosensor-a-dedicated-daemon-to-report-gpio-status-.patch
@@ -1,7 +1,8 @@
-From 2ed7f3bdd3900102d8dd333bc2c6ad6c2ef83688 Mon Sep 17 00:00:00 2001
+From cebbcd890ac479429811fc2c5858f8e13099e8a3 Mon Sep 17 00:00:00 2001
From: Gaurav Gandhi <gauravgandhi@google.com>
Date: Mon, 13 Feb 2023 18:23:56 -0800
-Subject: [PATCH] gpiosensor: a dedicated daemon to report gpio status to dbus
+Subject: [PATCH 1/2] gpiosensor: a dedicated daemon to report gpio status to
+ dbus
The daemon will read the given gpio status and report to dbus. One can
consume the gpio signal and behave differently.
@@ -19,16 +20,17 @@
Signed-off-by: Cody Smith <scody@google.com>
In-Review: https://gerrit.openbmc-project.xyz/c/openbmc/dbus-sensors/+/45997/37
Change-Id: Ife901358751eac46b4a52a878feabcb556fb1224
+Signed-off-by: John Wedig <johnwedig@google.com>
---
- docs/gpio-based-presence-detection.md | 120 +++++++++++
- include/GPIOPresenceSensor.hpp | 110 ++++++++++
+ docs/gpio-based-presence-detection.md | 120 ++++++++++
+ include/GPIOPresenceSensor.hpp | 110 +++++++++
meson.options | 1 +
service_files/meson.build | 1 +
- .../xyz.openbmc_project.gpiosensor.service | 13 ++
- src/GPIOPresenceSensor.cpp | 105 ++++++++++
- src/GPIOPresenceSensorMain.cpp | 188 ++++++++++++++++++
+ .../xyz.openbmc_project.gpiosensor.service | 13 +
+ src/GPIOPresenceSensor.cpp | 105 ++++++++
+ src/GPIOPresenceSensorMain.cpp | 225 ++++++++++++++++++
src/meson.build | 16 ++
- 8 files changed, 554 insertions(+)
+ 8 files changed, 591 insertions(+)
create mode 100644 docs/gpio-based-presence-detection.md
create mode 100644 include/GPIOPresenceSensor.hpp
create mode 100644 service_files/xyz.openbmc_project.gpiosensor.service
@@ -433,15 +435,19 @@
+} // namespace gpiopresencesensing
diff --git a/src/GPIOPresenceSensorMain.cpp b/src/GPIOPresenceSensorMain.cpp
new file mode 100644
-index 0000000..8b22d25
+index 0000000..02c7ed5
--- /dev/null
+++ b/src/GPIOPresenceSensorMain.cpp
-@@ -0,0 +1,188 @@
+@@ -0,0 +1,225 @@
+#include "Utils.hpp"
+
++#include <boost/asio/steady_timer.hpp>
+#include <GPIOPresenceSensor.hpp>
+
++#include <chrono>
+#include <iostream>
++#include <mutex>
++#include <utility>
+
+namespace gpiopresencesensing
+{
@@ -474,6 +480,62 @@
+ /*present*/ false, pollRate};
+}
+
++void handleExistingDevices(sdbusplus::asio::connection* conn,
++ OnInterfaceAddedCallback&& cb, size_t retries)
++{
++ conn->async_method_call(
++ [conn, callback = std::move(cb), retries]
++ (const boost::system::error_code ec, ManagedObjectType managedObjs) {
++ if (ec)
++ {
++ std::cerr << "Error getting Entity Manager managed objects; "
++ << retries << " retries remaining." << std::endl;
++ if (retries == 0)
++ {
++ return;
++ }
++ auto timer = std::make_shared<boost::asio::steady_timer>(
++ conn->get_io_context());
++ timer->expires_after(std::chrono::seconds(10));
++ timer->async_wait(
++ [conn, callback = std::move(callback), retries, timer]
++ (boost::system::error_code ec) mutable {
++ if (ec)
++ {
++ std::cerr << "Timer error!";
++ return;
++ }
++ handleExistingDevices(
++ conn,
++ std::move(callback),
++ retries - 1);
++ });
++ return;
++ }
++ for (auto& obj : managedObjs)
++ {
++ auto& item = obj.second;
++ auto found = item.find(interfaces::emGPIOCableSensingIfc);
++ if (found != item.end())
++ {
++ Config config;
++ try
++ {
++ config = getConfig(found->second);
++ callback(obj.first.str, found->first, config);
++ }
++ catch (std::exception& e)
++ {
++ std::cerr << "Incomplete config found: " << e.what()
++ << " obj = " << obj.first.str << std::endl;
++ }
++ }
++ }
++ },
++ "xyz.openbmc_project.EntityManager", "/xyz/openbmc_project/inventory",
++ "org.freedesktop.DBus.ObjectManager", "GetManagedObjects");
++}
++
+void setupInterfaceAdded(sdbusplus::asio::connection* conn,
+ OnInterfaceAddedCallback&& cb)
+{
@@ -500,35 +562,7 @@
+ };
+
+ // call the user callback for all the device that is already available
-+ conn->async_method_call(
-+ [cb](const boost::system::error_code ec,
-+ ManagedObjectType managedObjs) {
-+ if (ec)
-+ {
-+ return;
-+ }
-+ for (auto& obj : managedObjs)
-+ {
-+ auto& item = obj.second;
-+ auto found = item.find(interfaces::emGPIOCableSensingIfc);
-+ if (found != item.end())
-+ {
-+ Config config;
-+ try
-+ {
-+ config = getConfig(found->second);
-+ cb(obj.first.str, found->first, config);
-+ }
-+ catch (std::exception& e)
-+ {
-+ std::cerr << "Incomplete config found: " << e.what()
-+ << " obj = " << obj.first.str << std::endl;
-+ }
-+ }
-+ }
-+ },
-+ "xyz.openbmc_project.EntityManager", "/xyz/openbmc_project/inventory",
-+ "org.freedesktop.DBus.ObjectManager", "GetManagedObjects");
++ handleExistingDevices(conn, std::forward<OnInterfaceAddedCallback>(cb), 5);
+
+ ifcAdded = std::make_unique<sdbusplus::bus::match::match>(
+ static_cast<sdbusplus::bus::bus&>(*conn),
@@ -593,6 +627,11 @@
+ [&controller, &systemBus,
+ &objectServer](std::string_view, std::string_view,
+ const gpiopresencesensing::Config& config) {
++ static std::mutex callbackMutex;
++ // Make sure we don't have multiple threads running this callback.
++ // Otherwise, it could try to add the same dbus object multiple times.
++ const std::lock_guard<std::mutex> lock(callbackMutex);
++
+ sdbusplus::message::object_path inventoryPath(
+ gpiopresencesensing::inventoryObjPath);
+ sdbusplus::message::object_path objPath = inventoryPath / config.name;
@@ -653,5 +692,5 @@
executable(
'externalsensor',
--
-2.44.0.rc1.240.g4c46232300-goog
+2.50.0.rc2.696.g1fc2a0284f-goog
diff --git a/recipes-phosphor/sensors/dbus-sensors/0004-Feature-Add-association-interfaces-to-a-target-EM.patch b/recipes-phosphor/sensors/dbus-sensors/0004-Feature-Add-association-interfaces-to-a-target-EM.patch
index 8b1f3dd..385d5db 100644
--- a/recipes-phosphor/sensors/dbus-sensors/0004-Feature-Add-association-interfaces-to-a-target-EM.patch
+++ b/recipes-phosphor/sensors/dbus-sensors/0004-Feature-Add-association-interfaces-to-a-target-EM.patch
@@ -1,7 +1,7 @@
-From febe200fdf9115bd327a86ad621e963def1b5f09 Mon Sep 17 00:00:00 2001
+From 9735639177b72d30adff67957fbd13fb0313e97e Mon Sep 17 00:00:00 2001
From: Cody Smith <scody@google.com>
Date: Fri, 18 Feb 2022 13:20:45 +0000
-Subject: [PATCH] Feature: Add association interfaces to a target EM
+Subject: [PATCH 2/2] Feature: Add association interfaces to a target EM
This change gives the `gpiopresence` daemon in `dbus-sensors`
the ability to, upon request via configuration EM, generate am
@@ -32,10 +32,10 @@
In-Review: https://gerrit.openbmc-project.xyz/c/openbmc/dbus-sensors/+/51360/12
Change-Id: I0572d124a8e9b9fedbdfb30d41fe399e005f5c03
---
- include/GPIOPresenceSensor.hpp | 17 +++++-
+ include/GPIOPresenceSensor.hpp | 17 ++++++-
src/GPIOPresenceSensor.cpp | 4 +-
- src/GPIOPresenceSensorMain.cpp | 94 +++++++++++++++++++++++++++++-----
- 3 files changed, 101 insertions(+), 14 deletions(-)
+ src/GPIOPresenceSensorMain.cpp | 88 ++++++++++++++++++++++++++++++----
+ 3 files changed, 98 insertions(+), 11 deletions(-)
diff --git a/include/GPIOPresenceSensor.hpp b/include/GPIOPresenceSensor.hpp
index a9f49a0..fa32a20 100644
@@ -51,7 +51,7 @@
+static constexpr inline const char* propertyAssociationReverse =
+ "AssociationReverse";
} // namespace Properties
-
+
namespace interfaces
@@ -50,8 +55,15 @@ struct Config
bool activeLow;
@@ -68,10 +68,10 @@
+ // (Internal) Parent path
+ std::string parentPath;
};
-
+
// Actively listen to the config information from EntityManager and calls the
@@ -65,9 +77,11 @@ class GPIOPresence
-
+
// Add a dbus object to the reference list.
// @params statusIfc: pointer to object status interface.
+ // @params associationIfc: Optional pointer to object association interface
@@ -80,7 +80,7 @@
void addObj(std::unique_ptr<sdbusplus::asio::dbus_interface> statusIfc,
+ std::unique_ptr<sdbusplus::asio::dbus_interface> associationIfc,
std::string_view objPath, const Config& config);
-
+
// Remove a object from the object reference list.
@@ -89,6 +103,7 @@ class GPIOPresence
struct ObjIfaces
@@ -89,13 +89,13 @@
+ std::unique_ptr<sdbusplus::asio::dbus_interface> associationIfc;
Config config;
};
-
+
diff --git a/src/GPIOPresenceSensor.cpp b/src/GPIOPresenceSensor.cpp
index 870b4b4..2d48491 100644
--- a/src/GPIOPresenceSensor.cpp
+++ b/src/GPIOPresenceSensor.cpp
@@ -26,10 +26,12 @@ int GPIOPresence::readLine(std::string_view lineLabel)
-
+
void GPIOPresence::addObj(
std::unique_ptr<sdbusplus::asio::dbus_interface> statusIfc,
+ std::unique_ptr<sdbusplus::asio::dbus_interface> associationIfc,
@@ -106,25 +106,22 @@
+ objIfaces[std::string(objPath)] = {std::move(statusIfc),
+ std::move(associationIfc), config};
}
-
+
void GPIOPresence::removeObj(std::string_view objPath)
diff --git a/src/GPIOPresenceSensorMain.cpp b/src/GPIOPresenceSensorMain.cpp
-index 8b22d25..f556900 100644
+index 02c7ed5..576f3c0 100644
--- a/src/GPIOPresenceSensorMain.cpp
+++ b/src/GPIOPresenceSensorMain.cpp
-@@ -1,8 +1,10 @@
- #include "Utils.hpp"
-
+@@ -2,6 +2,7 @@
+
+ #include <boost/asio/steady_timer.hpp>
#include <GPIOPresenceSensor.hpp>
+#include <sdbusplus/bus/match.hpp>
-
+
+ #include <chrono>
#include <iostream>
-+#include <utility>
-
- namespace gpiopresencesensing
- {
-@@ -15,7 +17,7 @@ using OnInterfaceRemovedCallback = std::function<void(std::string_view)>;
-
+@@ -19,7 +20,7 @@ using OnInterfaceRemovedCallback = std::function<void(std::string_view)>;
+
// Helper function to convert dbus property to struct
// @param[in] properties: dbus properties
-Config getConfig(const SensorBaseConfigMap& properties)
@@ -132,7 +129,7 @@
{
auto name = loadVariant<std::string>(properties, Properties::propertyName);
auto gpioLine =
-@@ -31,8 +33,48 @@ Config getConfig(const SensorBaseConfigMap& properties)
+@@ -35,8 +36,48 @@ Config getConfig(const SensorBaseConfigMap& properties)
pollRate =
loadVariant<uint32_t>(properties, Properties::propertyPollRate);
}
@@ -181,9 +178,28 @@
+ associationReverse,
+ std::move(parentPath)};
}
-
- void setupInterfaceAdded(sdbusplus::asio::connection* conn,
-@@ -49,7 +91,7 @@ void setupInterfaceAdded(sdbusplus::asio::connection* conn,
+
+ void handleExistingDevices(sdbusplus::asio::connection* conn,
+@@ -73,14 +114,16 @@ void handleExistingDevices(sdbusplus::asio::connection* conn,
+ }
+ for (auto& obj : managedObjs)
+ {
+- auto& item = obj.second;
++ SensorData& item = obj.second;
++
+ auto found = item.find(interfaces::emGPIOCableSensingIfc);
+ if (found != item.end())
+ {
+ Config config;
+ try
+ {
+- config = getConfig(found->second);
++ config = getConfig(found->second,
++ obj.first.parent_path());
+ callback(obj.first.str, found->first, config);
+ }
+ catch (std::exception& e)
+@@ -109,7 +152,7 @@ void setupInterfaceAdded(sdbusplus::asio::connection* conn,
Config config;
try
{
@@ -192,37 +208,7 @@
callback(objPath.str, found->first, config);
}
catch (std::exception& e)
-@@ -62,23 +104,24 @@ void setupInterfaceAdded(sdbusplus::asio::connection* conn,
-
- // call the user callback for all the device that is already available
- conn->async_method_call(
-- [cb](const boost::system::error_code ec,
-- ManagedObjectType managedObjs) {
-+ [callback = cb](const boost::system::error_code ec,
-+ ManagedObjectType managedObjs) {
- if (ec)
- {
- return;
- }
- for (auto& obj : managedObjs)
- {
-- auto& item = obj.second;
-+ SensorData& item = obj.second;
-+
- auto found = item.find(interfaces::emGPIOCableSensingIfc);
- if (found != item.end())
- {
- Config config;
- try
- {
-- config = getConfig(found->second);
-- cb(obj.first.str, found->first, config);
-+ config = getConfig(found->second, obj.first.parent_path());
-+ callback(obj.first.str, found->first, config);
- }
- catch (std::exception& e)
- {
-@@ -103,7 +146,7 @@ void setupInterfaceRemoved(sdbusplus::asio::connection* conn,
+@@ -135,7 +178,7 @@ void setupInterfaceRemoved(sdbusplus::asio::connection* conn,
OnInterfaceRemovedCallback&& cb)
{
// Listen to the interface removed event.
@@ -231,7 +217,7 @@
[callback = std::move(cb)](sdbusplus::message::message msg) {
sdbusplus::message::object_path objPath;
msg.read(objPath);
-@@ -162,14 +205,41 @@ int main()
+@@ -199,14 +242,41 @@ int main()
{
controller->removeObj(objPath.str);
}
@@ -275,6 +261,6 @@
controller->setMinPollRate(config.pollRate);
// It is possible there are more EM config in the pipeline.
// Therefore, delay the main loop by 10 seconds to wait for more
---
-2.40.1.521.gf1e218fcd8-goog
+--
+2.50.0.rc2.696.g1fc2a0284f-goog
