Manikandan Somasundaram

This commit **refines the hardware test suite** by **removing specific MERU platforms** (`MERU800BFA`, `MERU800BIA`) from the list of devices permitted an **extended setup time**. Previously, these platforms were granted a longer timeout during hardware initialization checks, but this exception is no longer necessary due to recent platform changes. The modification in `fboss/platform/platform_manager/hw_test/PlatformManagerHwTest.cpp` ensures that only `MORGAN800CC` now utilizes the extended setup time, thereby **tightening the validation criteria** for the affected MERU platforms. This **maintenance update** aligns the test behavior with current platform requirements, potentially leading to more rigorous and accurate hardware test results for these devices.

This commit **adds comprehensive unit tests** for all methods within the **`ParserUtils`** utility, located in `fboss/platform/weutil`. Specifically, tests are introduced for `calculateCrc16`, `parseBeUint`, `parseBeHex`, `parseString`, and `parseMac`. These new tests cover normal operation, various edge cases, and robust error handling scenarios, significantly improving the **reliability and correctness** of these parsing functions. A build dependency for the CRC16 utility was also added to `fboss/platform/weutil/test/BUCK` to support the new test suite.

This commit performs significant **refactoring and cleanup** within the **`fboss/platform/fan_service/Bsp`** module, enhancing its maintainability and correctness. It streamlines `Bsp.h` by removing unused includes, updating comments, and adjusting member visibility, specifically moving `setEmergencyState` to private. The `Bsp.cpp` implementation is improved by moving inline function definitions, removing unused variables, and promoting hardcoded class members to file-scope constants. Crucially, it **fixes an exception slicing bug** in `readSysfs` by changing `throw e` to `throw`, and enhances readability by replacing the magic number `255` with `kDefaultMaxBrightness`.

This commit **refactors** the **`fan_service`** component by extracting the `MediaInterfaceCode` to optic type mapping logic into a dedicated helper function. The `switch` statement previously embedded in `processOpticEntries()` within `fboss/platform/fan_service/Bsp.cpp` is now encapsulated in `getOpticTypeFromMediaCode()`, which takes an `std::optional<MediaInterfaceCode>` and returns `std::nullopt` for unrecognized types. This change significantly improves code modularity and readability by centralizing the mapping logic and simplifying the original call site by eliminating a ternary operator. Additionally, the function `getOpticsDataFromQsfpSvc` is **renamed** to `getOpticData` for better clarity and consistency.

This commit **fixes an integration test** for the **fan service** to correctly validate its behavior during **FSDB outages**. Specifically, it updates `FanSensorFsdbIntegrationTests.cpp` to account for a **thrift fallback mechanism** that ensures **sensor data remains available** and timestamps are updated even when FSDB is down. This **test update** removes an incorrect assertion about static timestamps, ensuring the **fan service's resilience** is properly verified. Additionally, it includes a minor **refactoring** of the `kFsdbSensorDataThriftFallback` constant and reduces test wait times.

This commit **adjusts a configuration parameter** within the `fboss/platform/platform_manager` subsystem to improve system stability. Specifically, it **increases the default platform exploration timeout** from 30 seconds to 40 seconds, as defined in `PlatformExplorer.h`. This **maintenance change** aims to reduce false positive alerts for setup time violations, allowing platforms sufficient time for initialization. The increased timeout ensures more reliable monitoring by preventing premature alerts and improving the accuracy of system health reporting.

This commit **implements a new capability to resolve I2C bus numbers for AMD FireRange CPUs** within the `fboss/platform/platform_manager` subsystem. It introduces `resolveAmdCpuBusNums()` in `I2cExplorer.cpp` to map ACPI firmware paths like `_SB_.I2CA` and `_SB_.I2CB` to virtual bus names `CPU_BUS@0` and `CPU_BUS@1`, respectively, addressing issues with identically named DesignWare I2C adapters. Additionally, the **`ConfigValidator` is updated to allow `CPU_BUS@1` and detect duplicate virtual bus names**, enhancing configuration robustness. This **new feature** is crucial for enabling correct I2C device communication and configuration on **AMD FireRange platforms** by accurately identifying their I2C buses.

This commit performs a significant **refactoring** of the `ControlLogic` class within the **fan service** to improve data flow and reduce unnecessary state. It removes the member variables `pSensor_`, `numFanFailed_`, and `numSensorFailed_`, which were only used locally, by explicitly passing their data as function parameters or return values. For instance, `SensorData` is now passed to `updateSensorPwms()` and `updateOpticsPwms()`, and `numSensorFailed` is returned from `updateSensorPwms()`. Additionally, `getSensorUpdate` and `getOpticsUpdate` have been **renamed** to `updateSensorPwms` and `updateOpticsPwms` respectively, enhancing the clarity and maintainability of the **fan control logic**.

This commit performs a **significant refactoring** by **removing the redundant `AccessMethod` abstraction** and its associated access type checks from the **`fan_service` platform component**. The abstraction was deemed unnecessary as all sensors consistently use THRIFT and all optics use QSFP, rendering the access type branching obsolete. This change involves removing the `AccessMethod` struct and access fields from Thrift definitions, along with related constants and validation. Consequently, it **simplifies the `Bsp` module's optics data retrieval** by eliminating unnecessary access type branching in `Bsp::getOpticsData()` and renaming `getOpticsDataFromQsfpSvc` to `getOpticData`. This effort reduces code complexity and improves maintainability within the `fan_service`.

This commit **refactors** the **fan service's emergency shutdown logic** by introducing **startup configuration validation**. It ensures that the `shutdownCmd` is properly defined whenever a `shutdownCondition` is specified, moving this check from a runtime sentinel value in `Bsp::emergencyShutdown()` to the `ConfigValidator::isValid()` function. This change improves system robustness by catching invalid configurations earlier and simplifies the `emergencyShutdown` implementation. Consequently, a related test case in `BspTests.cpp` for the old sentinel check was removed, reflecting the improved validation strategy.

This commit significantly **enhances the reliability** of the **`fan_service`** by implementing a **fallback mechanism** for sensor data acquisition. When `FLAGS_subscribe_to_stats_from_fsdb` is active, the system will now **fall back to direct Thrift** to `sensor_service` if **FSDB sensor data** is detected as stale (older than 3 minutes) or has never been received, ensuring continuous operation. This **new capability** prevents `fan_service` from operating with outdated or missing sensor information and includes **bug fixes** to improve **logging clarity** in `FsdbSensorSubscriber` and `ControlLogic` regarding unreceived or stale data. A new `sensor_data_fsdb_to_thrift_fallback` counter has also been added to track these critical fallback events.

This commit **refactors** the **fan service** to improve both code clarity and performance. Specifically, it optimizes **fan zone membership checks** within `ControlLogic.cpp` by replacing repeated `std::find` calls with efficient `std::unordered_set` lookups in `setTransitionValue()` and `updateControl()`, leading to **O(1) performance** for these operations. Additionally, the **configuration validation logic** in `ConfigValidator::isValidOpticConfig()` is simplified by merging duplicate validation blocks for PID and IncrementalPID aggregation types. These changes collectively enhance the **maintainability and efficiency** of the fan service's core logic and configuration handling.

This commit introduces several **bug fixes** and minor **refactoring** within the **fan service** module. It **corrects the agent data staleness check** in `FsdbSensorSubscriber.cpp` by validating `agentLastUpdatedTime` instead of `sensorStatsLastUpdatedTime`, ensuring accurate data freshness. A critical **gpiod resource leak** in `ControlLogic.cpp` is resolved using `SCOPE_EXIT` guards, preventing resource exhaustion during GPIO operations. Additionally, it fixes a comma operator bug in `calculateZonePwm` to ensure zone names are correctly logged in errors and initializes `SensorReadCache::lastUpdatedTime` to prevent unpredictable staleness checks. These changes significantly improve the **robustness, reliability, and diagnostic capabilities** of the fan service.

This commit performs significant **code cleanup and refactoring** within the **`fboss/platform/fan_service`** module by removing dead configuration fields and associated code paths. It eliminates unused `ACCESS_TYPE_SYSFS` and `ACCESS_TYPE_UTIL` constants, the `AccessMethod.path` field, and the `portList` from the `Optic` thrift struct, which were never utilized in any platform configurations. This **maintenance** effort streamlines the **Board Support Package (BSP)** by simplifying `Bsp::getSensorData()` and `Bsp::processOpticEntries()` logic, removing unnecessary checks and unreachable code. The overall impact is improved maintainability and reduced complexity within the platform's sensor and optic configuration handling, with corresponding test cases also removed.

This commit **optimizes platform setup time thresholds** by significantly reducing the `kMaxSetupTimeMorgan800CC` constant from 75s to 45s for the **Cisco morgan800cc platform**. This **maintenance** change leverages recent improvements in BSP upgrade times, which previously caused extended platform setup durations. The `fboss/platform/platform_manager/PlatformExplorer.h` module undergoes **refactoring** to introduce `kMaxSetupTimeViolaters` and adjust other `kMaxSetupTime` values, with corresponding updates in `PlatformManagerHwTest.cpp` to use these new constants. This adjustment allows for **faster detection of platform setup issues** by tightening the timeout, aligning it with the improved actual performance.

This commit introduces a **new capability** by adding the `getFirmwareVersions()` method to the **PlatformManagerService Thrift API**. This allows external clients to query **firmware version data** that is already collected by the `DataStore` from sources like `/run/devmap/cplds` and `/run/devmap/inforoms`. The change involves defining a new `FirmwareVersionsResponse` struct in `platform_manager_service.thrift`, declaring and implementing the method in `PlatformManagerHandler.h` and `PlatformManagerHandler.cpp` respectively, and adding comprehensive test cases in `PlatformManagerHandlerTest.cpp`. This enhances the **Platform Manager's queryable data** without altering its data collection mechanisms, following existing thread-safe patterns for data access.

This commit introduces **automatic configuration change detection** within the **`platform_manager`** to ensure **kernel modules (kmods) are reloaded** whenever its configuration has materially changed. This **new feature** enhances the system's operational robustness by automatically triggering kmod reloads, even if the `--reload_kmods` flag is not explicitly set. The implementation involves calling `hasConfigChanged()` at startup and persisting the configuration hash using `storeConfigHash()` after successful exploration, primarily affecting the `main` function in `fboss/platform/platform_manager/Main.cpp`. This ensures that the system consistently reflects the latest `platform_manager` configuration.

This commit performs a **configuration update** for the **`minipack3ba` platform manager**, standardizing its I2C bus identification. It replaces the specific hardware-dependent string "SMBus I801 adapter at 5000" with the **virtual and platform-agnostic identifier `CPU_BUS@0`** within the `busName` and `i2cAdaptersFromCpu` fields of `fboss/platform/configs/minipack3ba/platform_manager.json`. This **maintenance** task aligns `minipack3ba` with a more generalized naming convention, similar to `minipack3n`, improving the **robustness and maintainability** of the platform's I2C adapter configurations. The change ensures consistent and flexible I2C bus identification for the platform by abstracting away physical hardware details.

This commit **fixes a bug** in the **RMA Showtech tool** that prevented individual selection of disruptive functions via the `--details` CLI option. Previously, `getValidDetailNames()` appended `(disruptive)` to function names, causing CLI validation to fail; now it returns bare names, enabling granular selection while the `--disruptive` flag continues to gate execution. This **usability enhancement** also improves the **CLI help output** by grouping available details into safe and disruptive categories, providing clearer guidance to users. Additionally, a typo in the disruptive skip message was corrected.

This commit **updates the `minipack3n` platform configuration** by migrating its bus naming convention within `platform_manager.json`. Specifically, it replaces the physical bus identifier "SMBus I801 adapter at 5000" with the **new virtual naming scheme** "`CPU_BUS@0`". This **configuration update** is a **maintenance** task, making `minipack3n` the **first platform to adopt this standardized virtual bus naming**. The change within the `fboss/platform` subsystem aims to improve consistency and simplify bus identification across various hardware platforms.