Kelly Byrd
2023-Nov-15 14:50 UTC
[Nut-upsdev] What is the arduino sub-driver intended to be used for?
Making progress on this PR. I have question about storing and checking state in a subdriver HID Power Devices can a "UPS.PowerSummary.CapacityMode" value. According to the spec, the values are: 0: maH 1: mwH 2: percent (%) 3: Boolean support only (OK or failed) Grep'ing the source, nothing makes use of this now (some subdrivers have it mapped to unmapped.ups.powersummary.capacitymode or something similar) and many of the HID UPS drivers assume CapacityMode 2 (they interpret RemainingCapacity, WarningCapacityLimit, RemainingCapacityLimit as percent). My application (and the original sample sketch for the Arduino project also does this, so I'm fine with that. But, I was wondering if anyone could point out the right way in a NUT subdriver to have a function that handles RemainingCapacity do different things based on CapacityMode. I think I understand how to write my own function to handle values in arduino-hid.c, but I don't know how to check the state of a previously parsed value, especially one that doesn't have a place to be stored in the normal nut names variable hierarchy. Or maybe this is too much trouble and I'll just assume CapacityMode = 2. On Mon, Nov 13, 2023 at 7:41?AM Kelly Byrd <kbyrd at memcpy.com> wrote:> oh! Ok, I'll go look at all the HID objects reported by the sample > code @abratchik has on top-of-tree of his repo and see what makes sense to > add to NUT. PR coming this week. > > > On Sun, Nov 12, 2023 at 2:38?PM Jim Klimov <jimklimov+nut at gmail.com> > wrote: > >> Well, the intention generally is to use it as a driver, so the more >> abilities the merrier (PR would be welcome) :) >> >> Per comments to initial PR that added it, >> https://github.com/networkupstools/nut/pull/1044 : >> >> > The idea of introducing this driver is to enable creation and >> prototyping of smart UPS based on Arduino, which is inexpensive versatile >> platform very much suitable for custom fully open-source UPS development. >> >> I don't think it was supposed to end and be finished work at that point, >> but distractions happen... >> >> It might be worth asking @abratchik if he has something tinkered over >> this time and ready to upstream, as well. >> >> Jim Klimov >> >> On Sun, Nov 12, 2023, 22:46 Kelly Byrd <kbyrd at memcpy.com> wrote: >> >>> I posted earlier on this list about getting an Arduino Pro Micro using >>> https://github.com/abratchik/HIDPowerDevice in a simple sketch to >>> monitor a DIY UPS I have built. With the changes from that thread (I had a >>> PR merged), I was able to get usbhid-ups to recognize my device and could >>> setup NUT the rest of the way. My intended configuration is to run NUT on a >>> Raspberry Pi, have that device be master and then have clients of that NUT >>> install take action when ACPresent goes from true to false. >>> >>> The problem I ran into is `upsc diyups at locahost` was always showing >>> ups.status as "OB". I ran tests and confirmed the raw reports >>> included "UPS.PowerSummary.PresentStatus.ACPresent correctly showing 1 and >>> 0 as appropriate as well as correctly reporting the >>> UPS.PowerSummary.PresentStatus.Charging >>> and UPS.PowerSummary.PresentStatus.Discharging bitfields. >>> >>> I traced the problem down to drivers/arduino.c. Apparently the Arduino >>> subdriver doesn't have an entry >>> for "UPS.PowerSummary.PresentStatus.ACPresent" in it's arduino_hid2nut[] >>> definition. The only sections present are commented as: >>> /* USB HID PDC defaults */ >>> >>> I my local source tree, I made changes to these entries copied from >>> another USB HID-based subdriver, to add: >>> /* USB HID UPS Status*/ >>> {"BOOL", 0, 0, "UPS.PowerSummary.PresentStatus.Charging", NULL, >>> NULL, HU_FLAG_QUICK_POLL, charging_info}, >>> {"BOOL", 0, 0, "UPS.PowerSummary.PresentStatus.Discharging", >>> NULL, NULL, HU_FLAG_QUICK_POLL, discharging_info}, >>> {"BOOL", 0, 0, "UPS.PowerSummary.PresentStatus.NeedReplacement", >>> NULL, NULL, 0, replacebatt_info}, >>> {"BOOL", 0, 0, "UPS.PowerSummary.PresentStatus.ACPresent", NULL, >>> NULL, HU_FLAG_QUICK_POLL, online_info}, >>> >>> and now everything works as expected! My question for this list is why >>> there were not included in the Arduino subdriver already? AFAICT, the >>> author of the HIDPowerDevice Arduino library added the Arduino subdriver >>> via a PR a few years ago along with basic support for the common Arduino >>> VID:PID combinations in other parts of the code. But, that same library >>> definitely reports more than >>> UPS.PowerSummary.DelayBeforeShutdown, UPS.PowerSummary.DelayBeforeStartup, >>> etc. >>> and the example code he gives primarily demonstrates how to report >>> ACPresent, Charging, and also remaining run time. >>> >>> Am I missing something? Is there a runtime config based way to instruct >>> NUT how to map ACPresent, Charging, etc HID statuses to NUT's specific >>> structure? Over on the GitHub for the other project, others have reported >>> success with NUT and Arduinos using that library, but I don't know what >>> "success" means to them. I can't see how it's actually useful without the >>> addition of the above lines. I'm probably missing the original intent. >>> >>> Any insight or advice is appreciated. If I'm on the right track, I'll do >>> another PR to add the above status flags (and others you all think are >>> important) to Arduino.c >>> >>> _______________________________________________ >>> Nut-upsdev mailing list >>> Nut-upsdev at alioth-lists.debian.net >>> https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/nut-upsdev >>> >>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://alioth-lists.debian.net/pipermail/nut-upsdev/attachments/20231115/3f001a2d/attachment.htm>
Greg Troxel
2023-Nov-15 15:50 UTC
[Nut-upsdev] What is the arduino sub-driver intended to be used for?
Kelly Byrd <kbyrd at memcpy.com> writes:> Making progress on this PR. I have question about storing and checking > state in a subdriver > HID Power Devices can a "UPS.PowerSummary.CapacityMode" value. According to > the spec, the values are: > 0: maH > 1: mwH > 2: percent (%) > 3: Boolean support only (OK or failed) > > Grep'ing the source, nothing makes use of this now (some subdrivers have it > mapped to unmapped.ups.powersummary.capacitymode or something similar) and > many of the HID UPS drivers assume CapacityMode 2 (they interpret > RemainingCapacity, WarningCapacityLimit, RemainingCapacityLimit as > percent). My application (and the original sample sketch for the Arduino > project also does this, so I'm fine with that. > > But, I was wondering if anyone could point out the right way in a NUT > subdriver to have a function that handles RemainingCapacity do different > things based on CapacityMode. I think I understand how to write my own > function to handle values in arduino-hid.c, but I don't know how to check > the state of a previously parsed value, especially one that doesn't have a > place to be stored in the normal nut names variable hierarchy. > > Or maybe this is too much trouble and I'll just assume CapacityMode = 2.I am really unclear on all of this, but it sounds like the USB interface has the device define CapacityMode and then provide a value. So some devices might report in mWh and some in %. I guess some might even be switchable but I would expect each one is how it is. When mapping to nut, my view is that the point is to provide a common abstraction while at the same time providing full access -- and these two goals are not entirely compatible. I would say that nut probably should have different variables for the different modes, and read the mode field, and then write the content of RemainingCapacity into RemainingCapacityPercent if mode is 2. Just writing without checking risks some other UPS having its mAh remaining value put in a field that data consumers think is %. I could be way off, but if so hopefully that's obvious!
Greg Troxel
2023-Nov-15 16:52 UTC
[Nut-upsdev] What is the arduino sub-driver intended to be used for?
Kelly Byrd <kbyrd at memcpy.com> writes:> Making progress on this PR. I have question about storing and checking > state in a subdriver > HID Power Devices can a "UPS.PowerSummary.CapacityMode" value. According to > the spec, the values are: > 0: maH > 1: mwH > 2: percent (%) > 3: Boolean support only (OK or failed)Looking at HID code briefly (my UPS devices have serial ports..), I see the fundamental issue is that the code is structured as a mapping table where there are essentially pairs of HID names and nut names. That presumes fixed semantics for each HID variable. This is a bug in the HID spec; having a mode is an unnecessary architectural complication vs just having 4 sets of variables some of which might or might not be there. So, I think the thing to do is for you not to worry for now, and that we should open a bug that the nut mapping scheme is unsound. Probably the mapping code needs to learn to check mode, and to rewrite the variable name or use some other flag, so that you can write mappings that go from each logical variable restricted to the expected mode to a nut variable. This change is not reasonable to impose as a requirement for a new driver that is not doing this any worse than the already-existing drivers.