On Fri, Dec 19, 2014 at 8:55 AM, Martin Pieuchot <mpieuc...@nolizard.org> wrote: > On 19/12/14(Fri) 08:04, David Higgs wrote: >> 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. > > What you're saying is that querying HUB_BATTERY_PRESENT after 6 sensors > that depend on its value does not make sense, right?
Correct. > Well in this case I don't think that sensors should be queried in > reportID order. I even don't think that sensors should be queried at > all if we're going to discard the result anyway. It might not matter > that much right now, but if the number of queried sensors grow > s significantly we might end up waiting a lot of time in upd_refresh() > prior to updating anything. Querying by report_ID minimizes the number of requests, because multiple sensors can be batched into the same report_ID. I don't know if there are any rules that determine how items are batched, so there may be pathologically bad results. >> 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). > > But do you need to read the value of CapacityMode more than once, at the > begging or when you modify it? True. CapacityMode shouldn't automatically change and can be read once (and/or maybe updated) at attach-time. I see no reason to expose this setting for users to change. > Anyway, when we discussed that with Andre, I suggested to have a table > containing all sensors depending on an other one. This way you can > start by querying HUB_BATTERY_PRESENT and if it is present, query the > others. Another advantage is that it will simplify the code because > all the sensors in a table will be in the same page. This sounds good. I will revisit the Power/Battery spec and see what future sensors might require dependent behavior. > Since you're doing some great job with upd(4), here's another idea. Did > you consider updating the values of the sensors via an asynchronous > function and a callback? This way the thread does not need to way for > the previous queries. That would involve adding a new function, let's > say uhidev_get_report_async() that takes a custom callback since there's > no such API for the moment. This sounds like a good idea, but batched report_ID values can produce redundant USB queries or will require additional logic/caching to minimize this redundancy. I'm new to all this, and am not sure how to decide which trade-offs to make. Thanks. --david
