From 2b2fce49628a4a6abf1ba015b82d241d23cc1ba6 Mon Sep 17 00:00:00 2001
From: Matt Vandermeulen <matt@reenigne.net>
Date: Tue, 15 Oct 2024 13:30:48 -0300
Subject: [PATCH] oem/smc: simplify the drive data

Signed-off-by: Matt Vandermeulen <matt@reenigne.net>
---
 oem/smc/drive.go      | 94 +++++++++++++++++++++++--------------------
 oem/smc/drive_test.go |  4 +-
 2 files changed, 52 insertions(+), 46 deletions(-)

diff --git a/oem/smc/drive.go b/oem/smc/drive.go
index 28314a57..c143e93e 100644
--- a/oem/smc/drive.go
+++ b/oem/smc/drive.go
@@ -2,38 +2,28 @@ package smc
 
 import (
 	"encoding/json"
+	"errors"
 
+	"github.com/stmcginnis/gofish/common"
 	"github.com/stmcginnis/gofish/redfish"
 )
 
+// ErrActionNotSupported is returned when the requested OEM-specific action
+// does not appear to be supported. This might happen when a device is new
+// or upgraded to a new firmware that follows the DMTF standards.
+var ErrActionNotSupported = errors.New("oem-specific action unsupported")
+
 // Drive extends a redfish.Drive for additional OEM fields
 type Drive struct {
 	redfish.Drive
-	smc struct {
-		Oem     driveOem     `json:"Oem"`
-		Actions driveActions `json:"Actions"`
-	}
-}
 
-type driveOem struct {
-	Supermicro struct {
-		Temperature             int
-		PercentageDriveLifeUsed int
-		DriveFunctional         bool
-	} `json:"Supermicro"`
-}
-
-type driveTarget struct {
-	Target     string `json:"target"`
-	ActionInfo string `json:"@Redfish.ActionInfo"`
-}
+	// Fields from the SMC OEM section
+	temperature             int
+	percentageDriveLifeUsed int
+	driveFunctional         bool
 
-type driveActions struct {
-	redfish.DriveActions
-	Oem struct {
-		DriveIndicate    driveTarget `json:"#Drive.Indicate"`
-		SmcDriveIndicate driveTarget `json:"#SmcDrive.Indicate"`
-	} `json:"Oem"`
+	// indicateTarget is the uri to hit to change the light state
+	indicateTarget string
 }
 
 // FromDrive returns an OEM-extended redfish drive
@@ -41,48 +31,64 @@ func FromDrive(drive *redfish.Drive) (Drive, error) {
 	smcDrive := Drive{
 		Drive: *drive,
 	}
-	smcDrive.smc.Actions.DriveActions = drive.Actions
 
-	if err := json.Unmarshal(drive.Oem, &smcDrive.smc.Oem); err != nil {
-		return smcDrive, err
+	var t struct {
+		Oem struct {
+			Supermicro struct {
+				Temperature             int
+				PercentageDriveLifeUsed int
+				DriveFunctional         bool
+			} `json:"Supermicro"`
+		} `json:"Oem"`
+		Actions struct {
+			Oem struct {
+				DriveIndicate    common.ActionTarget `json:"#Drive.Indicate"`
+				SmcDriveIndicate common.ActionTarget `json:"#SmcDrive.Indicate"`
+			} `json:"Oem"`
+		} `json:"Actions"`
 	}
 
-	if err := json.Unmarshal(drive.Actions.Oem, &smcDrive.smc.Actions.Oem); err != nil {
+	// Populate the Oem data
+	if err := json.Unmarshal(drive.RawData, &t); err != nil {
 		return smcDrive, err
 	}
 
+	smcDrive.temperature = t.Oem.Supermicro.Temperature
+	smcDrive.percentageDriveLifeUsed = t.Oem.Supermicro.PercentageDriveLifeUsed
+	smcDrive.driveFunctional = t.Oem.Supermicro.DriveFunctional
+
+	// We check both the SmcDriveIndicate and the DriveIndicate targets
+	// in the Oem sections - certain models and bmc firmwares will mix
+	// these up, so we check both
+	smcDrive.indicateTarget = t.Actions.Oem.DriveIndicate.Target
+	if len(t.Actions.Oem.SmcDriveIndicate.Target) > 0 {
+		smcDrive.indicateTarget = t.Actions.Oem.SmcDriveIndicate.Target
+	}
+
 	return smcDrive, nil
 }
 
 // Temperature returns the OEM provided temperature for the drive
 func (d Drive) Temperature() int {
-	return d.smc.Oem.Supermicro.Temperature
+	return d.temperature
 }
 
 // PercentageDriveLifeUsed returns the OEM provided drive life estimate as a percentage used
 func (d Drive) PercentageDriveLifeUsed() int {
-	return d.smc.Oem.Supermicro.PercentageDriveLifeUsed
+	return d.percentageDriveLifeUsed
 }
 
 // Functional returns the OEM provided flag that suggests whether a drive is functional or not
 func (d Drive) Functional() bool {
-	return d.smc.Oem.Supermicro.DriveFunctional
-}
-
-// indicateTarget figures out what uri to follow for indicator light actions.
-// This is a separate function for testing.
-func (d Drive) indicateTarget() string {
-	// We check both the SmcDriveIndicate and the DriveIndicate targets
-	// in the Oem sections - certain models and bmc firmwares will mix
-	// these up, so we check both
-	if len(d.smc.Actions.Oem.SmcDriveIndicate.Target) > 0 {
-		return d.smc.Actions.Oem.SmcDriveIndicate.Target
-	}
-
-	return d.smc.Actions.Oem.DriveIndicate.Target
+	return d.driveFunctional
 }
 
 // Indicate will set the indicator light activity, true for on, false for off
 func (d Drive) Indicate(active bool) error {
-	return d.Post(d.indicateTarget(), map[string]interface{}{"Active": active})
+	// Return a common error to let the user try falling back on the normal gofish path
+	if d.indicateTarget == "" {
+		return ErrActionNotSupported
+	}
+
+	return d.Post(d.indicateTarget, map[string]interface{}{"Active": active})
 }
diff --git a/oem/smc/drive_test.go b/oem/smc/drive_test.go
index 4635d793..e2d2f349 100644
--- a/oem/smc/drive_test.go
+++ b/oem/smc/drive_test.go
@@ -61,7 +61,7 @@ func TestSmcDriveOem(t *testing.T) {
 		t.Errorf("unexpected oem drive temerature: %d", smcDrive.Temperature())
 	}
 
-	if smcDrive.indicateTarget() != "/redfish/v1/Chassis/NVMeSSD.0.Group.0.StorageBackplane/Drives/Disk.Bay.22/Actions/Oem/Drive.Indicate" {
-		t.Errorf("unexpected oem drive indicator target: %s", smcDrive.indicateTarget())
+	if smcDrive.indicateTarget != "/redfish/v1/Chassis/NVMeSSD.0.Group.0.StorageBackplane/Drives/Disk.Bay.22/Actions/Oem/Drive.Indicate" {
+		t.Errorf("unexpected oem drive indicator target: %s", smcDrive.indicateTarget)
 	}
 }