skeleton: fixed hard-coding of master OCC hwmon sysfs path for PowerCap object

[adamliyi]

Currently master OCC hwmon sysfs path is hardcoded to '/sys/class/hwmon/hwmon3/'.
This make the '/org/openbmc/sensors/host/PowerCap' interface cannot work when
master OCC is initilized as hwmon device other than 'hwmon3'.

'usr_powercap' attribute should be searched when OCC is active.
When skeleton creates 'PowerCap' objects, OCC may not be active.
So do the search each time setting power cap. Raise an exception when
there is no 'user_powercap' attribute.

This would fix the issue: #42

Also raise an exception when setting power cap fails. The exception
will be returned to REST API caller. This would fix the issue:
#43

Signed-off-by: Yi Li adamliyi@msn.com

---

This change is

[adamliyi]

I will submit a new PR today based on Brad's review. Please hold a minute.

[amboar]

I've replied on-list with some comments - this was prior to reviewable.io being hooked up. Probably best to reply on-list?

---

Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from the review on Reviewable.io

[mdmillerii]

Should we be searching for the power cap attribute or can we know where it is?

Is the location of the Master OCC with the power cap fixed by the
FSI topology which is a system wiring attribute?

Should we be storing the user set value to apply on the next boot? If
so we should cache the min and max limits so the value could be set
while the system is not running.

---

Review status: all files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.

---

bin/Sensors.py, line 165 [r1] (raw file):
Not found is a TypeError ?

---

bin/Sensors.py, line 184 [r1] (raw file):
InvalidArgs is better , but is there a RangeError exception?

---

bin/Sensors.py, line 191 [r1] (raw file):
Again TypeError ?

---

bin/Sensors.py, line 206 [r1] (raw file):
What happens when the next system design connects the processors
to another i2c bus?

This seems hard coded to assume the one instance of a power cap
sensor will be the one for our system. Should we only search the
OCC directories belonging to the desired host? Can the master OCC
change for a given system or is it limited by the topology?

Can we express the expected location in the sensor config?

---

Comments from the review on Reviewable.io

[adamliyi]

Thanks for the comments.

The reason for searching HWMON attribute path each time the "PowerCap::setValue()" is called is that,
the kernel generates sysfs hwmon entry dynamically (when OCC becomes active). What is fixed is Master OCC's i2c address, on Barreleye, it is 0x50@i2c-3.

I just learned in an OCC training, that "set User Power cap" needs to handle the cases, when:

1. OCC is active
2. OCC is inactive
   There is a DCMI interface for setting Power limit: http://www.intel.com/content/www/us/en/data-center/dcmi/dcmi-v1-5-rev-spec.html
   Although we may not need to follow DCMI, the current set power cap function will need re-work to handle complicated situations, including saving "user set value", min and max power limit across reboot.

---

Review status: 1 of 2 files reviewed at latest revision, 4 unresolved discussions.

---

bin/Sensors.py, line 165 [r1] (raw file):
In the rest server: https://github.com/openbmc/phosphor-rest-server/blob/master/obmc-rest

The rest server returns error code to rest caller, by catching exception from dbus method.
DBUS_INVALID_ARGS = 'org.freedesktop.DBus.Error.InvalidArgs'
DBUS_TYPE_ERROR = 'org.freedesktop.DBus.Python.TypeError'
"
def do_post(self, path, method):
try:
if request.parameter_list:
return request.route_data'method'
else:
return request.route_data'method'

    except dbus.exceptions.DBusException, e:
        if e.get_dbus_name() == DBUS_INVALID_ARGS:
            abort(400, str(e))
        if e.get_dbus_name() == DBUS_TYPE_ERROR:
            abort(400, str(e))
        raise

"
So in order to get 400 error, I will need to choose between those two exceptions.

---

bin/Sensors.py, line 206 [r1] (raw file):
I agree that "3-0050" is hard coded here. It should be put into Barreleye specific configuration.

---

Comments from the review on Reviewable.io

[adamliyi]

I just updated the PR, removed the hard-coded "3-0050" i2c device address. It is not necessary, since only master occ has these power cap related sensors, and there is only one master occ in a system.

[mdmillerii]

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from Reviewable

[mdmillerii]

The current search seems ok as long as we don't need to support multiple p8 machines from one bmc.

[williamspatrick]

Please submit through Gerrit if needed.