Fix the looping behaviour in the getstatesensorreading command

This bug was actually caused by two host sensors that has the exact same entity id, instance id, container id but being used to implement different state sets. The old host code had two sensors on processor module that implements both stateset 1 & 11 that caused this regression and using this PR we have improved the sensor parsing abilities so that we can compare the state setId as well before deciding that a sensor is unique. Signed-off-by: Manojkiran Eda <[email protected]>
SrideviRamesh · Sep 1, 2021 · ddf6a1c · ddf6a1c
1 parent 2cb06bc
commit ddf6a1c
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 11 deletions.
diff --git a/common/types.hpp b/common/types.hpp
@@ -48,7 +48,8 @@ using PossibleStates = std::set<uint8_t>;
 //!< composite efffecter/sensor
 using CompositeSensorStates = std::vector<PossibleStates>;
 using EntityInfo = std::tuple<ContainerID, EntityType, EntityInstance>;
-using SensorInfo = std::tuple<EntityInfo, CompositeSensorStates>;
+using SensorInfo =
+    std::tuple<EntityInfo, CompositeSensorStates, std::vector<StateSetId>>;
 
 } // namespace pdr
 

diff --git a/host-bmc/host_pdr_handler.cpp b/host-bmc/host_pdr_handler.cpp
@@ -4,6 +4,7 @@
 
 #include "libpldm/fru.h"
 #include "libpldm/requester/pldm.h"
+#include "libpldm/state_set.h"
 #include "oem/ibm/libpldm/fru.h"
 
 #include "custom_dbus.hpp"
@@ -780,18 +781,21 @@ void HostPDRHandler::setHostSensorState(const PDRList& stateSensorPDRs,
                         pldm::pdr::EntityInfo entityInfo{};
                         pldm::pdr::CompositeSensorStates
                             compositeSensorStates{};
+                        std::vector<pldm::pdr::StateSetId> stateSetIds{};
 
                         try
                         {
-                            std::tie(entityInfo, compositeSensorStates) =
+                            std::tie(entityInfo, compositeSensorStates,
+                                     stateSetIds) =
                                 lookupSensorInfo(sensorEntry);
                         }
                         catch (const std::out_of_range& e)
                         {
                             try
                             {
                                 sensorEntry.terminusID = PLDM_TID_RESERVED;
-                                std::tie(entityInfo, compositeSensorStates) =
+                                std::tie(entityInfo, compositeSensorStates,
+                                         stateSetIds) =
                                     lookupSensorInfo(sensorEntry);
                             }
                             catch (const std::out_of_range& e)
@@ -1149,7 +1153,9 @@ void HostPDRHandler::setOperationStatus()
         {
             pldm::pdr::EntityInfo entityInfo{};
             pldm::pdr::CompositeSensorStates compositeSensorStates{};
-            std::tie(entityInfo, compositeSensorStates) = sensor.second;
+            std::vector<pldm::pdr::StateSetId> stateSetIds{};
+            std::tie(entityInfo, compositeSensorStates, stateSetIds) =
+                sensor.second;
             const auto& [containerId, entityType, entityInstance] = entityInfo;
 
             if (node.entity_type != entityType ||
@@ -1159,9 +1165,17 @@ void HostPDRHandler::setOperationStatus()
                 continue;
             }
             uint8_t state = 0;
-            // Get sensorOpState property by the getStateSensorReadings command.
-            getPresentStateBySensorReadigs(sensor.first.sensorID, state,
-                                           sensorMapIndex->first);
+            if (compositeSensorStates.size() == 1 &&
+
+                stateSetIds[0] == PLDM_STATE_SET_OPERATIONAL_FAULT_STATUS)
+            {
+                // set the dbus property only when its not a composite sensor
+                // and the state set it PLDM_STATE_SET_OPERATIONAL_FAULT_STATUS
+                // Get sensorOpState property by the getStateSensorReadings
+                // command.
+                getPresentStateBySensorReadigs(sensor.first.sensorID, state,
+                                               sensorMapIndex->first);
+            }
         }
     }
 }

diff --git a/libpldmresponder/pdr_utils.cpp b/libpldmresponder/pdr_utils.cpp
@@ -150,6 +150,7 @@ std::tuple<TerminusHandle, SensorID, SensorInfo>
     CompositeSensorStates sensors{};
     auto statesPtr = pdr->possible_states;
     auto compositeSensorCount = pdr->composite_sensor_count;
+    std::vector<StateSetId> stateSetIds{};
 
     while (compositeSensorCount--)
     {
@@ -173,6 +174,8 @@ std::tuple<TerminusHandle, SensorID, SensorInfo>
                       updateStates);
 
         sensors.emplace_back(std::move(possibleStates));
+        stateSetIds.emplace_back(state->state_set_id);
+
         if (compositeSensorCount)
         {
             statesPtr += sizeof(state_sensor_possible_states) +
@@ -184,8 +187,8 @@ std::tuple<TerminusHandle, SensorID, SensorInfo>
         std::make_tuple(static_cast<ContainerID>(pdr->container_id),
                         static_cast<EntityType>(pdr->entity_type),
                         static_cast<EntityInstance>(pdr->entity_instance));
-    auto sensorInfo =
-        std::make_tuple(std::move(entityInfo), std::move(sensors));
+    auto sensorInfo = std::make_tuple(std::move(entityInfo), std::move(sensors),
+                                      std::move(stateSetIds));
     return std::make_tuple(pdr->terminus_handle, pdr->sensor_id,
                            std::move(sensorInfo));
 }

diff --git a/libpldmresponder/platform.cpp b/libpldmresponder/platform.cpp
@@ -409,10 +409,11 @@ int Handler::sensorEvent(const pldm_msg* request, size_t payloadLength,
 
         pldm::pdr::EntityInfo entityInfo{};
         pldm::pdr::CompositeSensorStates compositeSensorStates{};
+        std::vector<pldm::pdr::StateSetId> stateSetIds{};
 
         try
         {
-            std::tie(entityInfo, compositeSensorStates) =
+            std::tie(entityInfo, compositeSensorStates, stateSetIds) =
                 hostPDRHandler->lookupSensorInfo(sensorEntry);
         }
         catch (const std::out_of_range& e)
@@ -423,7 +424,7 @@ int Handler::sensorEvent(const pldm_msg* request, size_t payloadLength,
             try
             {
                 sensorEntry.terminusID = PLDM_TID_RESERVED;
-                std::tie(entityInfo, compositeSensorStates) =
+                std::tie(entityInfo, compositeSensorStates, stateSetIds) =
                     hostPDRHandler->lookupSensorInfo(sensorEntry);
             }
             // If there is no mapping for events return PLDM_SUCCESS