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)