Split upd_update_sensors() behavior to gather all values prior to updating 
sensor contents.

Because sensors were updated in report_ID order, it could take multiple passes 
of upd_refresh() to update some sensors.  Specifically, battery-related sensors 
“prior” to a change in BatteryPresent would be stale/incorrect until the 2nd 
call to upd_refresh().  Also, change a confusing DPRINTF of report_ID from 
un-prefixed hex to decimal, to be consistent.

This fix is needed for future improvements, where output or behavior of certain 
sensors depends on the current value of more than one report (e.g. 
RemainingCapacity should actually depend on CapacityMode).

As always, feedback is welcome.

--david


Index: upd.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/upd.c,v
retrieving revision 1.12
diff -u -p -r1.12 upd.c
--- upd.c       11 Dec 2014 18:50:32 -0000      1.12
+++ upd.c       19 Dec 2014 05:33:14 -0000
@@ -77,6 +77,8 @@ struct upd_report {
 struct upd_sensor {
        struct ksensor          ksensor;
        struct hid_item         hitem;
+       int                     raw_value;
+       int                     value_read;
        int                     attached;
 };
 
@@ -98,7 +100,8 @@ void upd_attach(struct device *, struct 
 int  upd_detach(struct device *, int);
 
 void upd_refresh(void *);
-void upd_update_sensors(struct upd_softc *, uint8_t *, unsigned int, int);
+void upd_update_values(struct upd_softc *, uint8_t *, unsigned int, int);
+void upd_update_sensors(struct upd_softc *);
 void upd_intr(struct uhidev *, void *, uint);
 struct upd_usage_entry *upd_lookup_usage_entry(const struct hid_item *);
 struct upd_sensor *upd_lookup_sensor(struct upd_softc *, int, int);
@@ -275,7 +278,7 @@ upd_refresh(void *arg)
                    UHID_FEATURE_REPORT, repid, buf, report->size);
 
                if (actlen == -1) {
-                       DPRINTF(("upd: failed to get report id=%02x\n", repid));
+                       DPRINTF(("upd: failed to get report id=%d\n", repid));
                        continue;
                }
 
@@ -283,8 +286,10 @@ upd_refresh(void *arg)
                if (actlen < report->size)
                        report->size = actlen;
 
-               upd_update_sensors(sc, buf, report->size, repid);
+               upd_update_values(sc, buf, report->size, repid);
        }
+
+       upd_update_sensors(sc);
 }
 
 struct upd_usage_entry *
@@ -318,33 +323,48 @@ upd_lookup_sensor(struct upd_softc *sc, 
 }
 
 void
-upd_update_sensors(struct upd_softc *sc, uint8_t *buf, unsigned int len,
+upd_update_values(struct upd_softc *sc, uint8_t *buf, unsigned int len,
     int repid)
 {
        struct upd_sensor       *sensor;
-       ulong                   hdata, batpres;
-       ulong                   adjust;
        int                     i;
 
+       for (i = 0; i < sc->sc_num_sensors; i++) {
+               sensor = &sc->sc_sensors[i];
+               if (sensor->hitem.report_ID == repid && sensor->attached) {
+                       sensor->value_read = 1;
+                       sensor->raw_value =
+                           hid_get_data(buf, len, &sensor->hitem.loc);
+                       DPRINTF(("%s: repid=%d, %s value=%d\n",
+                           sc->sc_sensordev.xname, repid,
+                           sensor->ksensor.desc, sensor->raw_value));
+               }
+       }
+}
+
+void
+upd_update_sensors(struct upd_softc *sc)
+{
+       struct upd_sensor       *sensor;
+       ulong                   adjust;
+       int                     i, batpres = 0;
+
        sensor = upd_lookup_sensor(sc, HUP_BATTERY, HUB_BATTERY_PRESENT);
-       batpres = sensor ? sensor->ksensor.value : -1;
+       if (sensor && sensor->value_read)
+               batpres = sensor->raw_value;
 
        for (i = 0; i < sc->sc_num_sensors; i++) {
                sensor = &sc->sc_sensors[i];
-               if (!(sensor->hitem.report_ID == repid && sensor->attached))
+               sensor->ksensor.flags |= SENSOR_FINVALID;
+               sensor->ksensor.status = SENSOR_S_UNKNOWN;
+               if (!sensor->value_read)
                        continue;
 
-               /* invalidate battery dependent sensors */
+               /* ignore battery sensors when no battery present */
                if (HID_GET_USAGE_PAGE(sensor->hitem.usage) == HUP_BATTERY &&
-                   batpres <= 0) {
-                       /* exception to the battery sensor itself */
-                       if (HID_GET_USAGE(sensor->hitem.usage) !=
-                           HUB_BATTERY_PRESENT) {
-                               sensor->ksensor.status = SENSOR_S_UNKNOWN;
-                               sensor->ksensor.flags |= SENSOR_FINVALID;
-                               continue;
-                       }
-               }
+                   HID_GET_USAGE(sensor->hitem.usage) != HUB_BATTERY_PRESENT &&
+                   !batpres)
+                       continue;
 
                switch (HID_GET_USAGE(sensor->hitem.usage)) {
                case HUB_REL_STATEOF_CHARGE:
@@ -358,16 +378,16 @@ upd_update_sensors(struct upd_softc *sc,
                        break;
                }
 
-               hdata = hid_get_data(buf, len, &sensor->hitem.loc);
-
-               sensor->ksensor.value = hdata * adjust;
+               sensor->ksensor.value = sensor->raw_value * adjust;
                sensor->ksensor.status = SENSOR_S_OK;
                sensor->ksensor.flags &= ~SENSOR_FINVALID;
-               DPRINTF(("%s: hidget data: %lu\n",
-                   sc->sc_sensordev.xname, hdata));
        }
-}
 
+       /* all values processed */
+       for (i = 0; i < sc->sc_num_sensors; i++) {
+               sc->sc_sensors[i].value_read = 0;
+       }
+}
 
 void
 upd_intr(struct uhidev *uh, void *p, uint len)


Reply via email to