Platform HLD

[GarrickHe]

• Add HLD for platform command

Signed-off-by: Garrick He [email protected]

[FuzailBrcm]

Hi Garrick,

The DB stores the airflow directions as "Intake" or "exhaust". As mentioned in the base class,
sonic-platform-common/sonic_platform_base/fan_base.py

# Possible fan directions (relative to port-side of device)
  FAN_DIRECTION_INTAKE = "intake"
  FAN_DIRECTION_EXHAUST = "exhaust" 

When I am printing the values in CLICK CLIs, I am printing the string as 'Intake' or 'Exhaust'. (using the capitalize() built-in method for strings).

Can you use "Intake" or "Exhaust" here too instead of 'fan_direction_exhaust' or 'fan_direction_intake'?

[GarrickHe]

Hi @fk410167
The DB actually stores airflow direction the way it's printed in the HLD:

admin@sonic:~$ redis-cli -n 6 HGETALL "FAN_INFO|FAN 7"
 1) "presence"
 2) "true"
 3) "status"
 4) "true"
 5) "speed"
 6) "6231"
 7) "target_speed"
 8) "0"
 9) "speed_tolerance"
10) "20"
11) "status_led"
12) "green"
13) "name"
14) "FanTray4-Fan1"
15) "direction"
16) "fan_direction_exhaust"         <=======
17) "serial"
18) "CN-03CH15-CES00-773-0016-A00"
19) "model"
20) "CN-03CH15-CES00-773-0016-A00"
admin@sonic:~$

Do you still want to change the output?

[FuzailBrcm]

I saw Dell platform's APIs and they return "fan_direction_exhaust" or "fan_direction_intake" which is written in the DB. This needs to be changed from Dell platform API side.

Regardless, if you print the airflow string obtained from DB using str.capitalize(), then output would be uniform in CLICK and KLISH.

[jeff-yin]

Currently consulting the platform team on this. We usually represent our fan directions as "forward" and "reverse", or "front-to-back" and v/v. I brought this up in a couple of meetings.

Anyway, it looks like this just prints whatever the platform API returns, so it could be an issue with what we're returning from the platform side and the dev team will follow up if there's an issue.

[FuzailBrcm]

Hi Garrick,

I am porting the community's 'show platform temperature' CLICK cli in braodcom_sonic_3.x and I see that they are also printing 'Warning' and 'Timestamp'.

admin@sonic:~$ show platform temperature
                  NAME    Temperature    High Th    Low Th    Crit High Th    Crit Low Th    Warning          Timestamp
----------------------  -------------  ---------  --------  --------------  -------------  ---------  -----------------
     Ambient ASIC Temp           37.0      100.0       N/A           120.0            N/A      False  20200302 06:58:57
 Ambient Fan Side Temp           28.5      100.0       N/A           120.0            N/A      False  20200302 06:58:57
Ambient Port Side Temp           31.0      100.0       N/A           120.0            N/A      False  20200302 06:58:57
       CPU Core 0 Temp           36.0       87.0       N/A           105.0            N/A      False  20200302 06:59:57
...

extra keys used are highlighted below,

9. "warning_status"
10. "False"
11. "critical_high_threshold"
12. "98.0"
13. "critical_low_threshold"
14. ""
15. "timestamp"
16. "20200609 15:33:41"

If we print these two keys in the o/p, it will be in-line with the community implemented CLI.

[GarrickHe]

I can add that in.

[FuzailBrcm]

Yes. Please add it.

[FuzailBrcm]

Hi Garrick,

Cant you just keep it as string?
In the community HLD which talks about these fields, I see it is taken as 'string'.

I actually made the code changes for KLISH, and tested locally. They seems to be working. 'warning_status' would be of type boolean where as 'timestamp' would be of the type string.

My code changes are under review as of now. One they are pushed, you could check them in dell code drop.

-Fuzail

[FuzailBrcm]

I meant keep it as string without following specific pattern. If we go for changing the pattern of the 'timestamp' field, we will be digressing from community code.

[GarrickHe]

@fk410167 - it's fine. I'll convert the string in the dB to be compatible with the YANG's pattern in the backend. The timestamp:
20200707 21:53:10

will always have a format like this in the db right?
format: YYYYDDMM HH:MM:SS

[FuzailBrcm]

Yes, the format will be like this only.

Before making the changes, please wait to check the changes I did. You will receive the changes in Dell code drop in couple of days. If you feel there is a need of conversion, you can do it then.

Thanks,

[GarrickHe]

@fk410167 - what changes have you made? Does it affect the format of the timestamp?

[FuzailBrcm]

No, just added the below leafs in the openconfig-platform-ext.yang.

  leaf warning-status {
        type boolean;
        description
            "Indication for temperature warning status";
    }

    leaf timestamp {
        type string;
        description "The timestamp for the fetched temperature";
    }

And in the backend, read the timestamp string from the db and store (& display) it as is.

[GarrickHe]

@fk410167 - So this code has been merged already? You could of at least given me a heads-up so I don't have to spend time on it since you told me to work on it. If the code hasn't been merged, send me an email because I have a .diff that uses the proper type for the timestamp and also I have the conversion function that converts to the proper OC Yang format for the timestamp.