Improve hardware telemetry error reporting by preserving canonical error codes. Previously, low-level telemetry errors were collapsed into generic unavailable states in the middleware cache, swallowing precise diagnostic details. This change: - Updates the telemetry accessor base to cache and expose the precise canonical status returned by hardware read operations. - Updates the polling sensor interface to map and propagate these granular status codes (such as driver read errors or data corruption) directly to the BMC state tables. - Ensures downstream interfaces (like Redfish) receive specific, machine-readable symptoms instead of generic failure flags. Testing: - Added unit tests verifying telemetry state transitions for valid data, driver read issues, and data integrity failures. Google-Bug-Id:511201651 PiperOrigin-RevId: 968842688 (cherry picked from commit 69630563cc324a007b4f3cf1c0af3201fe2079ea) Change-Id: Ie38f1a4ca6431e436eae5bdb009fdd62b4eb99ba
diff --git a/tlbmc/hal/nic_veeprom/accessor_base.h b/tlbmc/hal/nic_veeprom/accessor_base.h index 89cfffb..fe4c0fe 100644 --- a/tlbmc/hal/nic_veeprom/accessor_base.h +++ b/tlbmc/hal/nic_veeprom/accessor_base.h
@@ -56,14 +56,14 @@ } absl::StatusOr<platforms::gbmc::hal::GnicVeepromData> data = gnic_telemetry_->ReadAndParse(); + absl::MutexLock lock(mutex_); + last_refresh_status_ = data.status(); if (!data.ok()) { LOG_EVERY_N_SEC(ERROR, 60) << "Failed to read and parse telemetry: " << data.status(); - absl::MutexLock lock(mutex_); sensor_values_ = std::nullopt; return; } - absl::MutexLock lock(mutex_); sensor_values_ = std::move(*data); last_refresh_timestamp_ = absl::Now(); } @@ -76,6 +76,7 @@ std::optional<platforms::gbmc::hal::GnicVeepromData> sensor_values_ ABSL_GUARDED_BY(mutex_); absl::Time last_refresh_timestamp_ ABSL_GUARDED_BY(mutex_); + absl::Status last_refresh_status_ ABSL_GUARDED_BY(mutex_) = absl::OkStatus(); }; template <typename T> @@ -108,6 +109,9 @@ << "VEEPROM data not available for sensor: " << NicTelemetryName_Name(name) << ". Last refresh: " << absl::FormatTime(last_refresh_timestamp_); + if (!last_refresh_status_.ok()) { + return last_refresh_status_; + } return absl::UnavailableError("VEEPROM data not available"); }
diff --git a/tlbmc/hal/nic_veeprom/interface.cc b/tlbmc/hal/nic_veeprom/interface.cc index f791003..967a563 100644 --- a/tlbmc/hal/nic_veeprom/interface.cc +++ b/tlbmc/hal/nic_veeprom/interface.cc
@@ -1,3 +1,4 @@ + #include "tlbmc/hal/nic_veeprom/interface.h" #include <memory> @@ -47,8 +48,10 @@ absl::string_view i2c_impl, absl::string_view eeprom_impl) { auto fallback_to_fake = [](const absl::Status& status) { LOG(WARNING) << "Falling back to FakeGnicTelemetry because: " << status; - return std::make_unique<internal::UnifiedAccessor>( + auto accessor = std::make_unique<internal::UnifiedAccessor>( std::make_unique<FakeGnicTelemetry>(status)); + accessor->DoRefresh(); + return accessor; }; absl::StatusOr<std::unique_ptr<platforms::gbmc::hal::I2cBus>> i2c_bus =
diff --git a/tlbmc/sensors/nic_sensor.cc b/tlbmc/sensors/nic_sensor.cc index d0bf730..581c1f8 100644 --- a/tlbmc/sensors/nic_sensor.cc +++ b/tlbmc/sensors/nic_sensor.cc
@@ -3,8 +3,10 @@ #include <memory> #include <optional> #include <string> +#include <utility> #include "absl/log/log.h" +#include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/match.h" #include "absl/strings/str_cat.h" @@ -18,6 +20,25 @@ #include "tlbmc/sensors/sync_polling_sensor.h" namespace milotic_tlbmc { +namespace { + +// Translates canonical HAL error codes into `milotic_tlbmc::Status` buckets. +milotic_tlbmc::Status MapNicSensorStatusToStoreStatus( + const absl::Status& hal_status) { + switch (hal_status.code()) { + case absl::StatusCode::kOk: + return milotic_tlbmc::STATUS_READY; + case absl::StatusCode::kUnavailable: + return milotic_tlbmc::STATUS_DRIVER_READ_ERROR; + case absl::StatusCode::kFailedPrecondition: + case absl::StatusCode::kDataLoss: + return milotic_tlbmc::STATUS_INVALID_DATA; + default: + return milotic_tlbmc::STATUS_OTHER_ERROR; + } +} + +} // namespace absl::StatusOr<std::shared_ptr<NicSensor>> NicSensor::Create( const NicTelemetryInstance& instance_config, @@ -60,6 +81,11 @@ absl::StatusOr<SensorValue> NicSensor::ReadSensorData() { absl::StatusOr<std::shared_ptr<const SensorValue>> result = accessor_.GetSensorValue(telemetry_name_); + milotic_tlbmc::Status store_status = + MapNicSensorStatusToStoreStatus(result.status()); + State state; + state.set_status(store_status); + this->UpdateState(std::move(state)); if (!result.ok()) { return absl::Status(result.status().code(), absl::StrCat("Failed to read sensor value with error: ",
diff --git a/tlbmc/sensors/nic_sensor.h b/tlbmc/sensors/nic_sensor.h index 55d2669..4ad9796 100644 --- a/tlbmc/sensors/nic_sensor.h +++ b/tlbmc/sensors/nic_sensor.h
@@ -3,7 +3,6 @@ #include <memory> #include <optional> -#include <string> #include "absl/status/statusor.h" #include "boost/asio.hpp" //NOLINT: boost::asio is commonly used in BMC
diff --git a/tlbmc/sensors/sync_polling_sensor.cc b/tlbmc/sensors/sync_polling_sensor.cc index dcf2070..68f3e7b 100644 --- a/tlbmc/sensors/sync_polling_sensor.cc +++ b/tlbmc/sensors/sync_polling_sensor.cc
@@ -52,8 +52,21 @@ start_time); if (!result.ok()) { - State state; - state.set_status(STATUS_STALE); + State state = sensor->GetSensorAttributesDynamic().state(); + // Case 1: The sensor implementation (e.g., NicSensor) already updated + // its dynamic state with a granular error status (DRIVER_READ_ERROR, + // INVALID_DATA, or OTHER_ERROR) before returning the error. In this + // case, we preserve the specific root-cause status. + // + // Case 2 (Otherwise): The sensor failed to read without setting a + // granular status (e.g., legacy sensors or tests where state is still + // STATUS_READY or STATUS_UNKNOWN). In this case, we fall back to + // STATUS_STALE so that a failing sensor is not reported as READY. + if (state.status() != STATUS_DRIVER_READ_ERROR && + state.status() != STATUS_INVALID_DATA && + state.status() != STATUS_OTHER_ERROR) { + state.set_status(STATUS_STALE); + } state.set_status_message(result.status().message()); sensor->UpdateState(std::move(state)); if (callback) {