sensorhandler: support setting analog sensors

Refactor YAML format to denote mutability of sensors.

Sensors which expect different formats for reads and writes
should present two entries in the sensor YAML, one with the read
interface and one with the write interface.  Sensors which
share a format for both reads and writes may present only one entry
in the YAML with both readable and writable enums specified.

If a sensor receives a write which has an interface of Sensor.Value,
the "Set" message is sent via DBus to the path provided in the YAML.
The previous codepath is maintained.

Change-Id: I292f95b6fe936de759fd65ce72c842a1bfe66448
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Patrick Venture <venture@google.com>

5 files changed, 196 insertions, 62 deletions

diff --git a/scripts/sensor-example.yaml b/scripts/sensor-example.yaml
index 2556261..52eb076 100755
--- a/scripts/sensor-example.yaml
+++ b/scripts/sensor-example.yaml
@@ -74,3 +74,20 @@
   sensorReadingType: 0x6F
   sensorType: 0xC3
   serviceInterface: org.freedesktop.DBus.Properties
+
+0xD0:
+  sensorType: 0x01
+  path: /xyz/openbmc_project/sensors/temperature/fleeting0
+  sensorReadingType: 0x01
+  multiplierM: 511
+  offsetB: 0
+  bExp: 0
+  mutability: Mutability::Write|Mutability::Read
+  serviceInterface: org.freedesktop.DBus.Properties
+  readingType: readingData
+  interfaces:
+    xyz.openbmc_project.Sensor.Value:
+      Value:
+        0xFF:
+          type: int64_t
+
diff --git a/scripts/writesensor.mako.cpp b/scripts/writesensor.mako.cpp
index e4609ab..5b1ffd7 100644
--- a/scripts/writesensor.mako.cpp
+++ b/scripts/writesensor.mako.cpp
@@ -48,6 +48,19 @@ inline ipmi_ret_t readingAssertion(const SetSensorReadingReq& cmdData,
 
 } // namespace sensor_${key}
 
+%elif "readingData" == readingType:
+
+namespace sensor_${key}
+{
+
+inline ipmi_ret_t readingData(const SetSensorReadingReq& cmdData,
+                              const Info& sensorInfo)
+{
+    return set::readingData<${type}>(cmdData, sensorInfo);
+}
+
+} // namespace sensor_${key}
+
 %endif
 % endfor
 
@@ -70,12 +83,15 @@ extern const IdInfoMap sensors = {
        updateFunc += sensor["readingType"]
        if "readingAssertion" == valueReadingType:
            updateFunc = "sensor_" + str(key) + "::" + valueReadingType
+       elif "readingData" == valueReadingType:
+           updateFunc = "sensor_" + str(key) + "::" + valueReadingType
        sensorInterface = serviceInterface
        if serviceInterface == "org.freedesktop.DBus.Properties":
            sensorInterface = next(iter(interfaces))
+       mutability = sensor.get("mutability", "Mutability::Read")
 %>
         ${sensorType},"${path}","${sensorInterface}",${readingType},${multiplier},
-        ${offset},${exp},${offset * pow(10,exp)},${updateFunc},{
+        ${offset},${exp},${offset * pow(10,exp)},${updateFunc},Mutability(${mutability}),{
     % for interface,properties in interfaces.items():
             {"${interface}",{
             % for dbus_property,property_value in properties.items():
@@ -107,7 +123,7 @@ extern const IdInfoMap sensors = {
             % endfor
             }},
     % endfor
-     }
+     },
 }},
    % endif
 % endfor
diff --git a/sensordatahandler.hpp b/sensordatahandler.hpp
index 68a122d..ca82666 100644
--- a/sensordatahandler.hpp
+++ b/sensordatahandler.hpp
@@ -95,6 +95,42 @@ ipmi_ret_t readingAssertion(const SetSensorReadingReq& cmdData,
     return updateToDbus(msg);
 }
 
+/** @brief Update d-bus based on a discrete reading
+ *  @param[in] cmdData - input sensor data
+ *  @param[in] sensorInfo - sensor d-bus info
+ *  @return an IPMI error code
+ */
+template<typename T>
+ipmi_ret_t readingData(const SetSensorReadingReq& cmdData,
+                       const Info& sensorInfo)
+{
+    auto msg = makeDbusMsg(
+                   "org.freedesktop.DBus.Properties",
+                   sensorInfo.sensorPath,
+                   "Set",
+                   sensorInfo.sensorInterface);
+
+    const auto& interface = sensorInfo.propertyInterfaces.begin();
+    msg.append(interface->first);
+
+    ipmi::sensor::Multiplier m = sensorInfo.coefficientM;
+    if (0 == m)
+    {
+        m = 1;  // Avoid * 0
+    }
+
+    // TODO: Refactor this into a generated function depending on the type
+    // of conversion for the value between IPMI and dbus.
+    T raw_value = (m * cmdData.reading) + sensorInfo.scaledOffset;
+
+    for (const auto& property : interface->second)
+    {
+        msg.append(property.first);
+        sdbusplus::message::variant<T> value = raw_value;
+        msg.append(value);
+    }
+    return updateToDbus(msg);
+}
 
 /** @brief Update d-bus based on eventdata type sensor data
  *  @param[in] cmdData - input sensor data
diff --git a/sensorhandler.cpp b/sensorhandler.cpp
index 4562476..b6e7890 100644
--- a/sensorhandler.cpp
+++ b/sensorhandler.cpp
@@ -2,6 +2,7 @@
 #include <math.h>
 #include <stdio.h>
 #include <string.h>
+#include <set>
 #include <bitset>
 #include <xyz/openbmc_project/Sensor/Value/server.hpp>
 #include <systemd/sd-bus.h>
@@ -408,6 +409,16 @@ ipmi_ret_t ipmi_sen_get_sensor_type(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
     return rc;
 }
 
+const std::set<std::string> analogSensorInterfaces =
+{
+    "xyz.openbmc_project.Sensor.Value",
+};
+
+bool isAnalogSensor(const std::string& interface)
+{
+    return (analogSensorInterfaces.count(interface));
+}
+
 ipmi_ret_t setSensorReading(void *request)
 {
     SetSensorReadingReq cmdData =
@@ -538,6 +549,13 @@ ipmi_ret_t ipmi_sen_get_sensor_reading(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
             }
 
             sensor = sensors.at(reqptr->sennum);
+            if (ipmi::sensor::Mutability::Read !=
+                  (sensor.mutability & ipmi::sensor::Mutability::Read))
+            {
+                log<level::ERR>("Sensor was not readable.\n");
+                return IPMI_CC_SENSOR_INVALID;
+            }
+
 
             // Get value
             r = sd_bus_get_property_trivial(bus,
@@ -710,70 +728,75 @@ ipmi_ret_t ipmi_sen_reserve_sdr(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
     return IPMI_CC_OK;
 }
 
-ipmi_ret_t populate_record_from_dbus(get_sdr::SensorDataFullRecordBody *body,
-                                     const ipmi::sensor::Info *info,
-                                     ipmi_data_len_t data_len)
+void setUnitFieldsForObject(sd_bus *bus,
+                            const dbus_interface_t &iface,
+                            const ipmi::sensor::Info *info,
+                            get_sdr::SensorDataFullRecordBody *body)
 {
-    /* Functional sensor case */
     if (info->propertyInterfaces.begin()->first ==
-            "xyz.openbmc_project.Sensor.Value")
+        "xyz.openbmc_project.Sensor.Value")
     {
-        // Get bus
-        sd_bus *bus = ipmid_get_sd_bus_connection();
-        dbus_interface_t iface;
-
-        if (0 > find_openbmc_path(body->entity_id, &iface))
-            return IPMI_CC_SENSOR_INVALID;
-
-        body->sensor_units_1 = 0; // unsigned, no rate, no modifier, not a %
-
-        /* Unit info */
-        char *raw_cstr;
+        std::string result {};
+        char *raw_cstr = NULL;
         if (0 > sd_bus_get_property_string(bus, iface.bus, iface.path,
                                             iface.interface, "Unit", NULL,
-                                            &raw_cstr)) {
-            fprintf(stderr, "Expected to find Unit interface in bus %s, path %s, but it was missing.\n",
-                    iface.bus, iface.path);
-            return IPMI_CC_SENSOR_INVALID;
+                                            &raw_cstr))
+        {
+            log<level::WARNING>("Unit interface missing.",
+                            entry("bus=%s, path=%s", iface.bus, iface.path));
+        }
+        else
+        {
+            result = raw_cstr;
         }
+        free(raw_cstr);
 
-        std::string raw_str(raw_cstr);
         namespace server = sdbusplus::xyz::openbmc_project::Sensor::server;
-        server::Value::Unit unit =
-            server::Value::convertUnitFromString(raw_str);
-
-        // Unit strings defined in
-        // phosphor-dbus-interfaces/xyz/openbmc_project/Sensor/Value.interface.yaml
-        switch (unit)
+        try {
+            auto unit = server::Value::convertUnitFromString(result);
+            // Unit strings defined in
+            // phosphor-dbus-interfaces/xyz/openbmc_project/Sensor/Value.interface.yaml
+            switch (unit)
+            {
+                case server::Value::Unit::DegreesC:
+                    body->sensor_units_2_base = get_sdr::SENSOR_UNIT_DEGREES_C;
+                    break;
+                case server::Value::Unit::RPMS:
+                    body->sensor_units_2_base = get_sdr::SENSOR_UNIT_REVOLUTIONS; // revolutions
+                    get_sdr::body::set_rate_unit(0b100, body); // per minute
+                    break;
+                case server::Value::Unit::Volts:
+                    body->sensor_units_2_base = get_sdr::SENSOR_UNIT_VOLTS;
+                    break;
+                case server::Value::Unit::Meters:
+                    body->sensor_units_2_base = get_sdr::SENSOR_UNIT_METERS;
+                    break;
+                case server::Value::Unit::Amperes:
+                    body->sensor_units_2_base = get_sdr::SENSOR_UNIT_AMPERES;
+                    break;
+                case server::Value::Unit::Joules:
+                    body->sensor_units_2_base = get_sdr::SENSOR_UNIT_JOULES;
+                    break;
+                default:
+                    // Cannot be hit.
+                    fprintf(stderr, "Unknown value unit type: = %s\n", result.c_str());
+            }
+        }
+        catch (sdbusplus::exception::InvalidEnumString e)
         {
-            case server::Value::Unit::DegreesC:
-                body->sensor_units_2_base = get_sdr::SENSOR_UNIT_DEGREES_C;
-                break;
-            case server::Value::Unit::RPMS:
-                body->sensor_units_2_base = get_sdr::SENSOR_UNIT_REVOLUTIONS; // revolutions
-                get_sdr::body::set_rate_unit(0b100, body); // per minute
-                break;
-            case server::Value::Unit::Volts:
-                body->sensor_units_2_base = get_sdr::SENSOR_UNIT_VOLTS;
-                break;
-            case server::Value::Unit::Meters:
-                body->sensor_units_2_base = get_sdr::SENSOR_UNIT_METERS;
-                break;
-            case server::Value::Unit::Amperes:
-                body->sensor_units_2_base = get_sdr::SENSOR_UNIT_AMPERES;
-                break;
-            case server::Value::Unit::Joules:
-                body->sensor_units_2_base = get_sdr::SENSOR_UNIT_JOULES;
-                break;
-            default:
-                fprintf(stderr, "Unknown value unit type: = %s\n", raw_cstr);
+            log<level::WARNING>("Warning: no unit provided for sensor!");
         }
+    }
+}
 
-        free(raw_cstr);
-
-        /* Modifiers to reading info */
-        // Get scale
-        int64_t scale;
+int64_t getScaleForObject(sd_bus *bus,
+                          const dbus_interface_t& iface,
+                          const ipmi::sensor::Info *info)
+{
+    int64_t result = 0;
+    if (info->propertyInterfaces.begin()->first ==
+        "xyz.openbmc_project.Sensor.Value")
+    {
         if (0 > sd_bus_get_property_trivial(bus,
                                         iface.bus,
                                         iface.path,
@@ -781,11 +804,37 @@ ipmi_ret_t populate_record_from_dbus(get_sdr::SensorDataFullRecordBody *body,
                                         "Scale",
                                         NULL,
                                         'x',
-                                        &scale)) {
-            fprintf(stderr, "Expected to find Scale interface in bus %s, path %s, but it was missing.\n",
-                    iface.bus, iface.path);
-            return IPMI_CC_SENSOR_INVALID;
+                                        &result)) {
+            log<level::WARNING>("Scale interface missing.",
+                            entry("bus=%s, path=%s", iface.bus, iface.path));
         }
+    }
+
+    return result;
+}
+
+ipmi_ret_t populate_record_from_dbus(get_sdr::SensorDataFullRecordBody *body,
+                                     const ipmi::sensor::Info *info,
+                                     ipmi_data_len_t data_len)
+{
+    /* Functional sensor case */
+    if (isAnalogSensor(info->propertyInterfaces.begin()->first))
+    {
+        // Get bus
+        sd_bus *bus = ipmid_get_sd_bus_connection();
+        dbus_interface_t iface;
+
+        if (0 > find_openbmc_path(body->entity_id, &iface))
+            return IPMI_CC_SENSOR_INVALID;
+
+        body->sensor_units_1 = 0; // unsigned, no rate, no modifier, not a %
+
+        /* Unit info */
+        setUnitFieldsForObject(bus, iface, info, body);
+
+        /* Modifiers to reading info */
+        // Get scale
+        int64_t scale = getScaleForObject(bus, iface, info);
 
         get_sdr::body::set_b(info->coefficientB, body);
         get_sdr::body::set_m(info->coefficientM, body);
diff --git a/types.hpp b/types.hpp
index 9c2237f..ae88faa 100644
--- a/types.hpp
+++ b/types.hpp
@@ -32,7 +32,6 @@ namespace sensor
 {
 
 using Offset = uint8_t;
-using Value = ipmi::Value;
 
 struct Values
 {
@@ -42,10 +41,8 @@ struct Values
 
 using OffsetValueMap = std::map<Offset,Values>;
 
-using DbusProperty = ipmi::DbusProperty;
 using DbusPropertyMap = std::map<DbusProperty,OffsetValueMap>;
 
-using DbusInterface = ipmi::DbusInterface;
 using DbusInterfaceMap = std::map<DbusInterface,DbusPropertyMap>;
 
 using InstancePath = std::string;
@@ -56,6 +53,24 @@ using OffsetB = uint16_t;
 using Exponent = uint8_t;
 using ScaledOffset = int64_t;
 
+enum class Mutability
+{
+   Read = 1 << 0,
+   Write = 1 << 1,
+};
+
+inline Mutability operator|(Mutability lhs, Mutability rhs)
+{
+  return static_cast<Mutability>(
+      static_cast<uint8_t>(lhs) | static_cast<uint8_t>(rhs));
+}
+
+inline Mutability operator&(Mutability lhs, Mutability rhs)
+{
+  return static_cast<Mutability>(
+      static_cast<uint8_t>(lhs) & static_cast<uint8_t>(rhs));
+}
+
 struct Info
 {
    Type sensorType;
@@ -67,6 +82,7 @@ struct Info
    Exponent exponentB;
    ScaledOffset scaledOffset;
    std::function<uint8_t(SetSensorReadingReq&, const Info&)> updateFunc;
+   Mutability mutability;
    DbusInterfaceMap propertyInterfaces;
 };