-
-
Notifications
You must be signed in to change notification settings - Fork 333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New driver hwmon_ina219 #2430
New driver hwmon_ina219 #2430
Conversation
Fixes: a816677 ("data/driver.list.in: add CP1350PFCLCD") Signed-off-by: Jan Viktorin <jan.viktorin@gmail.com>
Looks interesting, thanks! I am not sure about classifying it as an "I2C" driver though, as it has no actual dependency on Maybe this warrants a new category, or placement into I can quite envision a future with a generic driver that would have a mapping in configuration - which NUT data points can be found in which sysfs-style filename (and perhaps a scaling factor for general approach), not unlike DMF as proposed in forks earlier for SNMP and USB HID drivers, and so the same But I guess the current tested implementation can stay as is (notably to constrain builds and delivery to Linux out of the box at the moment), and the ideas above addressed/generalized later. |
Would you have a data dump (e.g. an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quite good as noted before. Got a few comments and proposed changes while reading the code. Also, can you please add a docs/man/hwmon_ina219.txt
man page, even if quite a simplistic one, for consistency and to document the few options the driver has (and perhaps to note that currently it only "sees" two readings from hardware, the momentary voltage and the current)? Your greatly detailed commit comment is a good start for large parts of the man page (especially about Raspberry OS image preparation).
Actually, with the added comment below about magic numbers for voltage=>charge conversion, when/if that becomes configurable, warrants being documented too (e.g. where to get those numbers, like cat
this path on a charged system).
Also, a NEWS.adoc
line to welcome the new driver would be useful :)
|
||
/** | ||
* @brief Voltage value as recently read from in1_input. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to comment that it would (may?) be in milliVolts / milliAmps (below) so you would shift it by 3 orders of magnitude into standard units range.
In fact, is it "guaranteed" that the values from sysfs would be integers and in milliUnits? Would it make sense to parse them in file_read_number()
as e.g. double
and range-check (if over 1000 then shift else use as is)? Just something to think about, I have no answers here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to comment that it would (may?) be in milliVolts / milliAmps (below) so you would shift it by 3 orders of magnitude into standard units range.
Mentioned in the doc comment.
In fact, is it "guaranteed" that the values from sysfs would be integers and in milliUnits? Would it make sense to parse them in file_read_number() as e.g. double and range-check (if over 1000 then shift else use as is)? Just something to think about, I have no answers here :)
Yes. From kernel doc, it is clearly stated that the miliUnits are used [1]. In sysfs interface standard [2], it is stated that "All sysfs values are fixed point numbers." You can find the print format "%d" in the kernel source code [3].
[1] https://docs.kernel.org/hwmon/ina2xx.html#general-sysfs-entries
[2] https://docs.kernel.org/hwmon/sysfs-interface.html#naming-and-data-format-standards-for-sysfs-files
[3] https://github.com/torvalds/linux/blob/7367539ad4b0f8f9b396baf02110962333719a48/drivers/hwmon/ina2xx.c#L300C16-L300C33
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Maybe it's worth pointing out in code comments, on one hand to avoid similar questions in the future, and on another to help eventual portability (whether to some newer Linux kernel 10.x convention or perhaps to extend this driver to more OSes with concepts similar to hwmon
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added few links there.
Hey guys, sorry to bother you with a noob question: I can't get the demo script provided by Waveshare to read the INA219 on the CM4-UPS-POE-BASE board... What I2C bus do you guys reach it on? On my bus 1, I don't see ANY I2C addresses, and on bus 20 and 21 there seem to be loads (none of them accessible though)?!? Help or even hints greatly appreciated... |
I am not sure what exactly do you mean. Simple the output?
|
Thanks for review. I'll address the comments soon.
Sure, I didn't know where exactly put which information so I made the commit message longer. It should be probably moved out as you suggest.
Well, I just copied what I found in the example code for CM4-POE-UPS-BASE 👼. I'll try to figure out the magic behind that.
👍 |
You have to enable the appropriate I2C controller, have you done it? It took me a while to find it out, it is not clear from the waveshare description. You can find the necessary steps (edit boot/config.txt and include device tree overlay) in the commit message 80756e1. |
This looks like life-saving intel Jan. Thx a million, will try this asap! |
✅ Build nut 2.8.2.1712-master completed (commit bf2feb8b81 by @jviki) |
I've followed these clear instructions to the letter, amended config.txt, manually created the .dts, placed it into /boot/overlays, unfortunately to no avail... |
Unfortunately, on my device, the i2c devices are not exposed this way and are only accessible by hwmon. And the device is in use, so I cannot run the same experiments. But, the outputs are very strange, I remember that I could clearly see few addresses on each bus. Outputs from my system:
On the i2c bus 10, you can find those devices as well:
From https://www.waveshare.com/wiki/CM4-POE-UPS-BASE:
Your setup looks like you are using the camera or DSI screen and thus you configured for this situation. In that case, the DTS overlay I included in the commit might not be right. I do not understand, what are those i2c-20 and i2c-21. What OS do you run on your system? |
Thank you so much for sharing your reflections! To make sure I'm not conflicting with previous settings attempts, I have installed a fresh Raspberry Pi OS Lite (bookworm), then literally just activated I2C in raspi-config and installed i2ctools and smbus. This listed me buses 1, 20 and 21 as shown in my screenshot. I saw that the demo code from Waveshare also mentioned/used address 43 on bus 10. I have no idea where that bus 10 would come from.... I've tried contacting Waveshare support, but they didn't come further than "advising me to install smbus". Do not feel obliged to lose your time on me. I'm not expert enough to figure things out without proper documentation. My bad... |
Do not give that easily. Another output that you can compare to:
The Can you post these outputs?
The appropriate DTS sources for bookworm should be located here: https://elixir.bootlin.com/linux/v6.1/source/arch/arm/boot/dts. The It is about finding out, what is enabled in the |
I noticed that my config.txt had dtoverlay=i2c_arm=on, and your has i2c_vc=on... |
It finally did come online... AND IT DID THE TRICK!!! With _vc instead of _arm, I now have: ... and 0x43 is present on bus 10! And my python script can read it!
Thanks a million. Haven't completely understood the difference between _vc and _arm, but comparing to your config.txt made the difference! Thanks a million!!! |
Me neither. The only place I could find it is probably here: https://www.waveshare.com/wiki/CM4-POE-UPS-BASE#RTC. I am happy that it works for you.
Thank you for testing this. I have to add this note somewhere into the doc. And I hope you will enjoy this driver in the future instead of that ugly custom python script. |
I will immediately admit I focussed on the UPS function (and therefore its wiki section) before anything... |
4c21ceb
to
2611ee9
Compare
❌ Build nut 2.8.2.1715-master failed (commit 9da73e69dc by @jviki) |
23cba55
to
14adcd9
Compare
I moved part of the commit message into |
Now when
or when the particular
|
@jimklimov How can I fix the spell checker issues? |
Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
904be11
to
576c345
Compare
INA219 [1] is a monitor of current. It is accessible via I2C. In the Linux Kernel, it is integrated via hwmon subsystem (thus, the I2C is hidden). When INA219 is installed to monitor current of batteries, it can be used as a simple UPS. This driver is intended to be used with CM4-POE-UPS-BASE [2], a baseboard for Raspberry PI Compute Module 4. With default setting, using 3x3000 mAh batteries, ~30% per-CPU avg load, ~200 kB/s of reads and writes to SSD disk, the system withstood ~6 hours without power. When remaining ~15 %, it took another 6,5 minutes before it died (that's enough time to usually shutdown gracefully): 20240501 165801 100 NA NA [OL] NA NA 20240501 165831 96 NA NA [OB DISCHRG] NA NA ... 20240501 231306 21 NA NA [OB DISCHRG] NA NA 20240501 231336 19 NA NA [OB DISCHRG] NA NA 20240501 231406 19 NA NA [OB DISCHRG] NA NA 20240501 231436 19 NA NA [OB DISCHRG] NA NA 20240501 231506 17 NA NA [OB DISCHRG] NA NA 20240501 231536 17 NA NA [OB DISCHRG] NA NA 20240501 231606 17 NA NA [OB DISCHRG] NA NA 20240501 231636 14 NA NA [FSD OB DISCHRG LB] NA NA 20240501 231706 13 NA NA [FSD OB DISCHRG LB] NA NA 20240501 231736 13 NA NA [FSD OB DISCHRG LB] NA NA 20240501 231806 13 NA NA [FSD OB DISCHRG LB] NA NA 20240501 231836 10 NA NA [FSD OB DISCHRG LB] NA NA 20240501 231906 11 NA NA [FSD OB DISCHRG LB] NA NA 20240501 231936 11 NA NA [FSD OB DISCHRG LB] NA NA 20240501 232006 9 NA NA [FSD OB DISCHRG LB] NA NA 20240501 232036 8 NA NA [FSD OB DISCHRG LB] NA NA 20240501 232106 7 NA NA [FSD OB DISCHRG LB] NA NA 20240501 232136 5 NA NA [FSD OB DISCHRG LB] NA NA 20240501 232206 4 NA NA [FSD OB DISCHRG LB] NA NA 20240501 232236 2 NA NA [FSD OB DISCHRG LB] NA NA 20240501 232306 0 NA NA [FSD OB DISCHRG LB] NA NA [1] https://www.ti.com/lit/ds/symlink/ina219.pdf [2] https://www.waveshare.com/wiki/CM4-POE-UPS-BASE Signed-off-by: Jan Viktorin <jan.viktorin@gmail.com>
Current upsc output:
|
Thanks a lot for the changes, will read into them tomorrow :) |
Is this listing more satisfying? 😆 (I unplugged the power in and it now runs on battery only, see DISCHRG flag.)
|
…cription [networkupstools#2430] Clarified existing pre-set combos recognized by the driver code. Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…workupstools#2430] Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
Hm, duplicate
|
… an addvar() auto-documented feature [networkupstools#2430] Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…9() a bit more idiomatic and log a skipped non-ina219 location [networkupstools#2430] Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…ot re-evaluating during driver uptime [networkupstools#2430] Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
This seems really cool and innovative - just wanted to say thanks a lot for the efforts to everyone involved. 💯 |
Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…ls#2430] Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
Synced with current codebase to minimize surprises with upcoming merge. |
…kupstools#2430] * as a console app that tried to be a service - report the error not only in Event Log but also on console; * for other issues write the LastError code to Event Log; * always exit with error code Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…kupstools#2430] * as a console app that tried to be a service - report the error not only in Event Log but also on console; * for other issues write the LastError code to Event Log; * always exit with error code Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
Initiated as #2378.
The driver implements communication with INA219 module that is supported by the Linux Kernel via hwmon interface. It is assumed that the INA219 monitors current and voltage from batteries. The implementation is based on the INA219 installed on Raspberry PI CM4 baseboard https://www.waveshare.com/wiki/CM4-POE-UPS-BASE. The code is inspired by the section https://www.waveshare.com/wiki/CM4-POE-UPS-BASE#INA219_Battery_Level_Detection.
The driver was successfully tested and monitored by using
upslog
. The device was equipped with 3x3000 mAh Li-lon batteries. The plotted data:The system runs on the battery only. At about 18:36 (see the dot in the plot), the power was plugged shortly to test whether it is correctly detected:
The nut-monitor service successfully detected 15 % of battery and initiated shutdown:
The shutdown was intentionally suppressed to see how the battery behaves until it is fully depleted: