From 8e64d466ebd5a92ec7f500ddaa0148404acb5435 Mon Sep 17 00:00:00 2001
From: wangjue <jue.wang2@luxshare-ict.com>
Date: Wed, 13 Nov 2024 20:25:01 +0800
Subject: [PATCH] Fix issue that the reading value of dimm temp sensor is zero
 even dimm is not present

Signed-off-by: wangjue <jue.wang2@luxshare-ict.com>
---
 src/DIMMTempSensor.cpp     | 62 +++++++++++++++++++++++++++++++++-----
 src/DIMMTempSensor.hpp     | 19 +++++++-----
 src/DIMMTempSensorMain.cpp | 18 ++++++-----
 3 files changed, 75 insertions(+), 24 deletions(-)

diff --git a/src/DIMMTempSensor.cpp b/src/DIMMTempSensor.cpp
index 1399995..d290d63 100644
--- a/src/DIMMTempSensor.cpp
+++ b/src/DIMMTempSensor.cpp
@@ -44,9 +44,36 @@ extern "C"
 }
 
 constexpr const bool debug = false;
-
+constexpr auto dimmSensorInterface = "xyz.openbmc_project.Sensor.dimmLED";
+constexpr auto dimmSensorPath = "/xyz/openbmc_project/sensors/temperature/";
 #define PECI_MBX_INDEX_DDR_DIMM_TEMP 0x0E
 
+static void setupDimmLEDMatch(
+    std::vector<sdbusplus::bus::match_t>& matches, sdbusplus::bus_t& connection,
+    const std::string& sensorName, std::function<void(const std::string&)>&& callback)
+{
+    std::function<void(sdbusplus::message_t & message)> eventHandler =
+        [callback{std::move(callback)}](sdbusplus::message_t& message) {
+        std::string objectName;
+        boost::container::flat_map<std::string, std::variant<std::string>>
+                        values;
+        message.read(objectName, values);
+        auto findValue = values.find("LEDState");
+        if (findValue == values.end())
+        {
+            return;
+        }
+        std::string value = std::visit(VariantToStringVisitor(), findValue->second);
+
+        callback(value);
+    };
+    matches.emplace_back(connection,
+                        "type='signal',member='PropertiesChanged',path_namespace='" +
+                        std::string(dimmSensorPath) + std::string(sensorName) + "',arg0namespace='" +
+                        std::string(dimmSensorInterface) + "'",
+                        std::move(eventHandler));
+}
+
 DIMMTempSensor::DIMMTempSensor(
     std::shared_ptr<sdbusplus::asio::connection>& conn,
     boost::asio::io_context& io, const std::string& sensorName,
@@ -54,14 +81,14 @@ DIMMTempSensor::DIMMTempSensor(
     sdbusplus::asio::object_server& objectServer,
     std::vector<thresholds::Threshold>&& thresholdData, const double maxReading,
     const double minReading, const std::string& sensorType, const char* units,
-    const float pollRate, PowerState readState, uint8_t peciBus,
-    uint8_t peciAddr, uint8_t rank, double calibOffset) :
+    const float pollRate, PowerState readState, double calibOffset,
+    uint8_t peciBus, uint8_t peciAddr, uint8_t rank) :
     Sensor(boost::replace_all_copy(sensorName, " ", "_"),
            std::move(thresholdData), sensorConfiguration, "", false, false,
            maxReading, minReading, conn, readState),
-    sensorPollMs(static_cast<unsigned int>(pollRate * 1000)), peciBus(peciBus),
-    peciAddr(peciAddr), dimmRank(rank), calibOffset(calibOffset),
-    objectServer(objectServer), waitTimer(io)
+    sensorPollMs(static_cast<unsigned int>(pollRate * 1000)), calibOffset(calibOffset),
+    objectServer(objectServer), waitTimer(io), peciBus(peciBus), peciAddr(peciAddr),
+    dimmRank(rank), dimmSensorName(sensorName), dimmLEDState("OFF")
 {
     std::string interfacePath = "/xyz/openbmc_project/sensors/" + sensorType +
                                 "/" + name;
@@ -72,7 +99,6 @@ DIMMTempSensor::DIMMTempSensor(
     dimmLEDInterface = objectServer.add_interface(
         interfacePath, "xyz.openbmc_project.Sensor.dimmLED");
 
-    std::string dimmLEDState = "OFF";
     dimmLEDInterface->register_property(
                 "LEDState", dimmLEDState, [this](const std::string& newValue, std::string& oldValue) {
                     oldValue = newValue; return true;
@@ -180,7 +206,7 @@ void DIMMTempSensor::read()
             std::cerr << "timer error\n";
             return;
         }
-        if (readingStateGood())
+        if (readingStateGood() && dimmLEDState == "ON")
         {
             double temp = 0;
             int ret = getDIMMRegsInfoWord(&temp);
@@ -208,3 +234,23 @@ void DIMMTempSensor::read()
         read();
     });
 }
+
+void DIMMTempSensor::setupMatches(const std::string& sensorName)
+{
+    std::weak_ptr<DIMMTempSensor> weakRef = weak_from_this();
+    setupDimmLEDMatch(matches, *dbusConnection, sensorName,
+        [weakRef, sensorName](const std::string& value) {
+            auto self = weakRef.lock();
+            if (!self)
+            {
+                return;
+            }
+            self->dimmLEDState = value;
+            if constexpr (debug)
+            {
+                std::cout << sensorName << " LED State" << " changed to "
+                            << self->dimmLEDState << std::endl;
+            }
+        }
+    );
+}
diff --git a/src/DIMMTempSensor.hpp b/src/DIMMTempSensor.hpp
index 1f7d8d3..774ace2 100644
--- a/src/DIMMTempSensor.hpp
+++ b/src/DIMMTempSensor.hpp
@@ -10,7 +10,7 @@
 #include <string>
 #include <vector>
 
-struct DIMMTempSensor : public Sensor
+struct DIMMTempSensor : public Sensor, std::enable_shared_from_this<DIMMTempSensor>
 {
     DIMMTempSensor(std::shared_ptr<sdbusplus::asio::connection>& conn,
                    boost::asio::io_context& io, const std::string& name,
@@ -19,25 +19,28 @@ struct DIMMTempSensor : public Sensor
                    std::vector<thresholds::Threshold>&& thresholds,
                    const double maxReading, const double minReading,
                    const std::string& sensorType, const char* units,
-                   const float pollRate, PowerState readState, uint8_t peciBus,
-                   uint8_t peciAddr, uint8_t rank, double calibOffset);
+                   const float pollRate, PowerState readState, double calibOffset,
+                   uint8_t peciBus, uint8_t peciAddr, uint8_t rank);
     ~DIMMTempSensor() override;
 
     void checkThresholds(void) override;
     void read();
     void init();
-
+    void setupMatches(const std::string& sensorName);
     unsigned int sensorPollMs;
-
-    uint8_t peciBus;
-    uint8_t peciAddr;
-    uint8_t dimmRank;
     double calibOffset;
 
   private:
     int getDIMMRegsInfoWord(double* ps64data);
     int getDIMMRegsTemp(double* ps64data);
    // int getDIMMRegsPower(double* ps64data);
+
     sdbusplus::asio::object_server& objectServer;
     boost::asio::steady_timer waitTimer;
+    uint8_t peciBus;
+    uint8_t peciAddr;
+    uint8_t dimmRank;
+    std::string dimmSensorName;
+    std::string dimmLEDState;
+    std::vector<sdbusplus::bus::match_t> matches;
 };
diff --git a/src/DIMMTempSensorMain.cpp b/src/DIMMTempSensorMain.cpp
index 042659f..a8fc05c 100644
--- a/src/DIMMTempSensorMain.cpp
+++ b/src/DIMMTempSensorMain.cpp
@@ -44,9 +44,11 @@ static constexpr double minTempReading = -128;
 static constexpr double maxPowerReading = 500;
 static constexpr double minPowerReading = 0;
 
+std::vector<std::unique_ptr<sdbusplus::bus::match::match>> matches;
+
 void createSensors(
     boost::asio::io_context& io, sdbusplus::asio::object_server& objectServer,
-    boost::container::flat_map<std::string, std::unique_ptr<DIMMTempSensor>>&
+    boost::container::flat_map<std::string, std::shared_ptr<DIMMTempSensor>>&
         sensors,
     std::shared_ptr<sdbusplus::asio::connection>& dbusConnection)
 {
@@ -125,22 +127,22 @@ void createSensors(
 
                 if (name.ends_with("_Temp"))
                 {
-                    sensor = std::make_unique<DIMMTempSensor>(
+                    sensor = std::make_shared<DIMMTempSensor>(
                         dbusConnection, io, name, pathPair.first, objectServer,
                         std::move(sensorThresholds), maxTempReading,
-                        minTempReading, "temperature",
-                        sensor_paths::unitDegreesC, pollRate, readState,
-                        peciBus, peciAddr, dimmRank, 0);
+                        minTempReading, "temperature", sensor_paths::unitDegreesC,
+                        pollRate, readState, 0, peciBus, peciAddr, dimmRank);
                 }
                 else if (name.ends_with("_Power"))
                 {
-                    sensor = std::make_unique<DIMMTempSensor>(
+                    sensor = std::make_shared<DIMMTempSensor>(
                         dbusConnection, io, name, pathPair.first, objectServer,
                         std::move(sensorThresholds), maxPowerReading,
                         minPowerReading, "power", sensor_paths::unitWatts,
-                        pollRate, readState, peciBus, peciAddr, dimmRank, 0);
+                        pollRate, readState, 0, peciBus, peciAddr, dimmRank);
                 }
 
+                sensor->setupMatches(name);
                 sensor->init();
             }
         }
@@ -158,7 +160,7 @@ int main()
     objectServer.add_manager("/xyz/openbmc_project/sensors");
     systemBus->request_name("xyz.openbmc_project.DIMMTempSensor");
 
-    boost::container::flat_map<std::string, std::unique_ptr<DIMMTempSensor>>
+    boost::container::flat_map<std::string, std::shared_ptr<DIMMTempSensor>>
         sensors;
     boost::asio::post(
         io, [&]() { createSensors(io, objectServer, sensors, systemBus); });
-- 
2.34.1

