Skip to content

File system based sensors implementation for voltage and current sensors#426

Merged
bmridul merged 12 commits intosonic-net:masterfrom
bmridul:sensor_fs
Mar 4, 2024
Merged

File system based sensors implementation for voltage and current sensors#426
bmridul merged 12 commits intosonic-net:masterfrom
bmridul:sensor_fs

Conversation

@bmridul
Copy link
Copy Markdown
Collaborator

@bmridul bmridul commented Jan 8, 2024

A file system based sensors implementation for Sensormond. Platforms can provide the sensor definition
in a file and Sensormond can use that to monitor the sensors.

HLD Reference: sonic-net/SONiC#1394

Description

Chassis base class looks for the presence of the file for sensors definition and uses that to built a list of sensor
objects if available.

Motivation and Context

See HLD mentioned above. This provides a simple implementation for platform support for Sensormond if the platforms have only Sysfs based sensors.

How Has This Been Tested?

Unit tested and tested on target.

root@sonic:/usr/local/etc# cat sensors.yaml
voltage_sensors:

  • name: VP3P3_CPU_RTC
    sensor: '/sys/bus/i2c/devices/23-0064/hwmon/hwmon49/in9_input'
    high_thresholds: [ 3560, 3600, 3630 ]
    low_thresholds: [ 2970, 3000, 3040 ]
  • name: VP3P3_CPU_SATA
    sensor: '/sys/bus/i2c/devices/23-0064/hwmon/hwmon49/in7_input'
    high_thresholds: [ 3560, 3600, 3630 ]
    low_thresholds: [ 2970, 3003, 3040 ]

cisco@sonic:~$ show platform volt
Sensor Voltage High TH Low TH Crit High TH Crit Low TH Warning Timestamp


VP3P3_CPU_RTC 3300 mV 3600 3000 3630 3040 False 20231125 20:01:05
VP3P3_CPU_SATA 3286 mV 3600 3003 3630 3040 False 20231125 20:01:05

Additional Information (Optional)

@prgeor
Copy link
Copy Markdown
Collaborator

prgeor commented Jan 18, 2024

@Junchao-Mellanox @keboliu could you review

@bmridul bmridul requested a review from keboliu February 6, 2024 07:07
@bmridul
Copy link
Copy Markdown
Collaborator Author

bmridul commented Feb 6, 2024

@keboliu , Pls recheck

Comment on lines +70 to +102
def get_high_critical_threshold(self):
"""Returns the sensor critical high threshold value"""
return self.high_thresholds[2]

def get_low_critical_threshold(self):
"""Returns the sensor critical low threshold value"""
return self.low_thresholds[2]

def set_high_critical_threshold(self, value):
"""Sets the sensor critical high threshold value"""
self.high_thresholds[2] = value
return True

def set_low_critical_threshold(self, value):
"""Sets the sensor critical low threshold value"""
self.low_thresholds[2] = value
return True

def get_minimum_recorded(self):
"""Retrieves the minimum recorded sensor measurement"""
tmp = self.get_value()
if tmp is None:
return None
if self.minimum_sensor is None or tmp < self.minimum_sensor:
self.minimum_sensor = tmp
return self.minimum_sensor

def get_maximum_recorded(self):
"""Retrieves the maximum recorded sensor measurement"""
tmp = self.get_value()
if tmp is None:
return None
if self.maximum_sensor is None or tmp > self.maximum_sensor:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into this, this would imply creating the private members of this implementation be put in the abstract base class. I think these are better left here.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bmridul this part I am still not clear...why most of these APIs are not in base class given that it has nothing to do with FS implement or not

@xumia
Copy link
Copy Markdown
Contributor

xumia commented Feb 22, 2024

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bmridul bmridul merged commit 56921d8 into sonic-net:master Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants