Skip to content
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

Merged
merged 11 commits into from
May 15, 2024
Merged

New driver hwmon_ina219 #2430

merged 11 commits into from
May 15, 2024

Conversation

jviki
Copy link
Contributor

@jviki jviki commented May 2, 2024

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:

image

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:

20240501 183536 70 NA NA [OB DISCHRG] NA NA
20240501 183606 72 NA NA [OB DISCHRG] NA NA
20240501 183636 86 NA NA [OL CHRG HB] NA NA
20240501 183706 72 NA NA [OB DISCHRG] NA NA
20240501 183736 72 NA NA [OB DISCHRG] NA NA

The nut-monitor service successfully detected 15 % of battery and initiated shutdown:

# May 01 18:05:07 umbrel upsmon[1359]: UPS test@localhost on battery
# May 01 18:05:07 umbrel systemd[1]: Started Network UPS Tools - power device monitor and shutdown controller.
# May 01 18:36:12 umbrel upsmon[1359]: UPS test@localhost on line power
# May 01 18:36:47 umbrel upsmon[1359]: UPS test@localhost on battery
# May 01 23:16:29 umbrel upsmon[1359]: UPS test@localhost battery is low
# May 01 23:16:29 umbrel upsmon[1359]: Executing automatic power-fail shutdown
# May 01 23:16:29 umbrel upsmon[1359]: Auto logout and shutdown proceeding
# May 01 23:16:34 umbrel systemd[1]: nut-monitor.service: Succeeded.

The shutdown was intentionally suppressed to see how the battery behaves until it is fully depleted:

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

Fixes: a816677 ("data/driver.list.in: add CP1350PFCLCD")
Signed-off-by: Jan Viktorin <jan.viktorin@gmail.com>
@jimklimov
Copy link
Member

Looks interesting, thanks!

I am not sure about classifying it as an "I2C" driver though, as it has no actual dependency on libi2c and is based on essentially parsing strings from sysfs (whose backend module and hardware may be or not be using I2C - as a black box, as far as NUT is concerned).

Maybe this warrants a new category, or placement into NUTSW_DRIVERLIST. Technically this driver (or the likes of it) might not be constrained to specifically Linux (there are other OSes allowing to tap into kernel/module info as files) and/or INA219 chip (we essentially expect certain names for voltage/current/... readings).

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 nutdrv_hwmon binary would handle various devices flexibly.

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.

@jimklimov jimklimov added the Linux Some issues are specific to Linux as a platform label May 3, 2024
@jimklimov jimklimov added this to the 2.8.3 milestone May 3, 2024
@jimklimov
Copy link
Member

Would you have a data dump (e.g. an upsc report) to stash for comparisons later?

Copy link
Member

@jimklimov jimklimov left a 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 :)

drivers/hwmon_ina219.c Outdated Show resolved Hide resolved
drivers/hwmon_ina219.c Outdated Show resolved Hide resolved
drivers/hwmon_ina219.c Outdated Show resolved Hide resolved
drivers/hwmon_ina219.c Outdated Show resolved Hide resolved

/**
* @brief Voltage value as recently read from in1_input.
*/
Copy link
Member

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 :)

Copy link
Contributor Author

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

Copy link
Member

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).

Copy link
Contributor Author

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.

drivers/hwmon_ina219.c Outdated Show resolved Hide resolved
drivers/hwmon_ina219.c Outdated Show resolved Hide resolved
@3dti
Copy link

3dti commented May 3, 2024

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...

@jviki
Copy link
Contributor Author

jviki commented May 3, 2024

Would you have a data dump (e.g. an upsc report) to stash for comparisons later?

I am not sure what exactly do you mean. Simple the output?

battery.charge: 100
battery.charge.high: 75
battery.charge.low: 15
battery.current: 0.000
battery.voltage: 4.204
device.description: Bidirectional Current/Power Monitor With I2C Interface
device.mfr: Texas Instruments
device.model: INA219
device.type: ups
driver.debug: 0
driver.flag.allow_killpower: 0
driver.name: hwmon_ina219
driver.parameter.pollinterval: 2
driver.parameter.port: /dev/null
driver.parameter.synchronous: auto
driver.state: quiet
driver.version: 2.8.1.1
driver.version.internal: 0.1
ups.mfr: Texas Instruments
ups.model: INA219
ups.status: OL HB
ups.type: ups

@jviki
Copy link
Contributor Author

jviki commented May 3, 2024

Quite good as noted before. Got a few comments and proposed changes while reading the code. Also, can you please

Thanks for review. I'll address the comments soon.

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).

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.

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).

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.

Also, a NEWS.adoc line to welcome the new driver would be useful :)

👍

@jviki
Copy link
Contributor Author

jviki commented May 3, 2024

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...

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.

@3dti
Copy link

3dti commented May 3, 2024

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...

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!

@AppVeyorBot
Copy link

@3dti
Copy link

3dti commented May 5, 2024

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.

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...
I'm stuck at the same situation: enabling I2C in Interface Options gets me 3 buses: 1, 20 and 21. With or without the "manual .dts", i2cdetect is still not seeing the 0x43 address...
On which bus do you guys find the 0x43 INA219?? Are the appropriate pull-up resistors present on the board? Do they need (dip)switching on somewhere?
At this point, I'd already be happy to see the component, not even talking about implementing proper UPS functionality yet... Thanks again, you seem to previously have ran into the same problem as me and clearly solved it!!

cm4_vs_ina219

@jviki
Copy link
Contributor Author

jviki commented May 5, 2024

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.

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... I'm stuck at the same situation: enabling I2C in Interface Options gets me 3 buses: 1, 20 and 21. With or without the "manual .dts", i2cdetect is still not seeing the 0x43 address... On which bus do you guys find the 0x43 INA219?? Are the appropriate pull-up resistors present on the board? Do they need (dip)switching on somewhere? At this point, I'd already be happy to see the component, not even talking about implementing proper UPS functionality yet... Thanks again, you seem to previously have ran into the same problem as me and clearly solved it!!
cm4_vs_ina219

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:

$ uname -a
Linux system 5.10.103-v8+ #1529 SMP PREEMPT Tue Mar 8 12:26:46 GMT 2022 aarch64 GNU/Linux
$ cat /etc/os-release 
PRETTY_NAME="Debian GNU/Linux 10 (buster)"
NAME="Debian GNU/Linux"
VERSION_ID="10"
VERSION="10 (buster)"
VERSION_CODENAME=buster
ID=debian
HOME_URL="https://www.debian.org/"
SUPPORT_URL="https://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/
$ ls /sys/class/i2c-adapter/i2c-*
/sys/class/i2c-adapter/i2c-0:
delete_device  device  mux_device  name  new_device  of_node  power  subsystem  uevent

/sys/class/i2c-adapter/i2c-10:
10-002f  10-0043  10-0051  delete_device  device  mux_device  name  new_device  of_node  power  subsystem  uevent

/sys/class/i2c-adapter/i2c-22:
delete_device  device  i2c-0  i2c-10  name  new_device  of_node  power  subsystem  uevent
$ ls /sys/class/i2c-adapter/i2c-*/of_node
/sys/class/i2c-adapter/i2c-0/of_node:
'#address-cells'   name   phandle   reg  '#size-cells'

/sys/class/i2c-adapter/i2c-10/of_node:
'#address-cells'   emc2301@2f   ina219@43   name   pcf85063a@51   phandle   reg  '#size-cells'   status

/sys/class/i2c-adapter/i2c-22/of_node:
'#address-cells'   clock-frequency   clocks   compatible   interrupts   name   phandle   reg  '#size-cells'   status

On the i2c bus 10, you can find those devices as well:

  • RTC (PCF85063a) on i2c-10, address is 0x51 (7-bit address)
  • FAN (EMC2301) on i2c-10, address is 0x2f (7-bit address)

From https://www.waveshare.com/wiki/CM4-POE-UPS-BASE:

Note DSI and CSI are prohibited using RTC.
I2C-10 is used by default.
CSI and DSI are disabled by default. When using the camera and DSI, it will occupy three I2C devices: I2C-10, I2C-11, and I2C-0.

If you need to use the camera or DSI screen, it is recommended to switch the I2C pin of the INA219 to GPIO2/3, and then modify it in the demo:

#ina219 = INA219(i2c_bus=10,addr=0x43)
ina219 = INA219(i2c_bus=1,addr=0x43)

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?

@3dti
Copy link

3dti commented May 5, 2024

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:

$ uname -a
Linux system 5.10.103-v8+ #1529 SMP PREEMPT Tue Mar 8 12:26:46 GMT 2022 aarch64 GNU/Linux
$ cat /etc/os-release 
PRETTY_NAME="Debian GNU/Linux 10 (buster)"
NAME="Debian GNU/Linux"
VERSION_ID="10"
VERSION="10 (buster)"
VERSION_CODENAME=buster
ID=debian
HOME_URL="https://www.debian.org/"
SUPPORT_URL="https://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/
$ ls /sys/class/i2c-adapter/i2c-*
/sys/class/i2c-adapter/i2c-0:
delete_device  device  mux_device  name  new_device  of_node  power  subsystem  uevent

/sys/class/i2c-adapter/i2c-10:
10-002f  10-0043  10-0051  delete_device  device  mux_device  name  new_device  of_node  power  subsystem  uevent

/sys/class/i2c-adapter/i2c-22:
delete_device  device  i2c-0  i2c-10  name  new_device  of_node  power  subsystem  uevent
$ ls /sys/class/i2c-adapter/i2c-*/of_node
/sys/class/i2c-adapter/i2c-0/of_node:
'#address-cells'   name   phandle   reg  '#size-cells'

/sys/class/i2c-adapter/i2c-10/of_node:
'#address-cells'   emc2301@2f   ina219@43   name   pcf85063a@51   phandle   reg  '#size-cells'   status

/sys/class/i2c-adapter/i2c-22/of_node:
'#address-cells'   clock-frequency   clocks   compatible   interrupts   name   phandle   reg  '#size-cells'   status

On the i2c bus 10, you can find those devices as well:

  • RTC (PCF85063a) on i2c-10, address is 0x51 (7-bit address)
  • FAN (EMC2301) on i2c-10, address is 0x2f (7-bit address)

From https://www.waveshare.com/wiki/CM4-POE-UPS-BASE:

Note DSI and CSI are prohibited using RTC.
I2C-10 is used by default.
CSI and DSI are disabled by default. When using the camera and DSI, it will occupy three I2C devices: I2C-10, I2C-11, and I2C-0.

If you need to use the camera or DSI screen, it is recommended to switch the I2C pin of the INA219 to GPIO2/3, and then modify it in the demo:
#ina219 = INA219(i2c_bus=10,addr=0x43)
ina219 = INA219(i2c_bus=1,addr=0x43)

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 then followed the instructions you pointed me too, and simply nothing seems to have changed. My aim is to interact with the INA219 through our own python scripts.

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 have neither installed or tried to configure, nor do I need any screen or camera.

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...

@jviki
Copy link
Contributor Author

jviki commented May 5, 2024

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 then followed the instructions you pointed me too, and simply nothing seems to have changed. My aim is to interact with the INA219 through our own python scripts.

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:

$ cat /boot/config.txt 
# For more options and information see
# http://rpf.io/configtxt
# Some settings may impact device functionality. See link above for details

# uncomment if you get no picture on HDMI for a default "safe" mode
#hdmi_safe=1

# uncomment this if your display has a black border of unused pixels visible
# and your display can output without overscan
#disable_overscan=1

# uncomment the following to adjust overscan. Use positive numbers if console
# goes off screen, and negative if there is too much border
#overscan_left=16
#overscan_right=16
#overscan_top=16
#overscan_bottom=16

# uncomment to force a console size. By default it will be display's size minus
# overscan.
#framebuffer_width=1280
#framebuffer_height=720

# uncomment if hdmi display is not detected and composite is being output
#hdmi_force_hotplug=1

# uncomment to force a specific HDMI mode (this will force VGA)
#hdmi_group=1
#hdmi_mode=1

# uncomment to force a HDMI mode rather than DVI. This can make audio work in
# DMT (computer monitor) modes
#hdmi_drive=2

# uncomment to increase signal to HDMI, if you have interference, blanking, or
# no display
#config_hdmi_boost=4

# uncomment for composite PAL
#sdtv_mode=2

#uncomment to overclock the arm. 700 MHz is the default.
#arm_freq=800

# Uncomment some or all of these to enable the optional hardware interfaces
#dtparam=i2c_arm=on
#dtparam=i2s=on
#dtparam=spi=on
dtparam=i2c_vc=on
dtoverlay=i2c-rtc,pcf85063a,i2c_csi_dsi
dtoverlay=cm4io-fan,minrpm=500,maxrpm=4800,midtemp=52000,midtemp_hyst=2000,maxtemp=60000,maxtemp_hyst=2000
dtoverlay=i2c-ina219
#dtoverlay=i2c-fan,emc2301,i2c_csi_dsi

# Uncomment this to enable infrared communication.
#dtoverlay=gpio-ir,gpio_pin=17
#dtoverlay=gpio-ir-tx,gpio_pin=18

# Additional overlays and parameters are documented /boot/overlays/README

# Enable audio (loads snd_bcm2835)
#dtparam=audio=on

# Enable DRM VC4 V3D driver on top of the dispmanx display stack
dtoverlay=vc4-fkms-v3d
max_framebuffers=2

arm_64bit=1

The dtoverlay options refer to the DTBO file (overlays). You can always convert DTBO back to DTS source code using dtc -I dtb -O dts. It might be a little bit messy, but from that, you can discover, how does it connect with the device tree. You need to figure out the right nodes in the device tree. Which are enabled, which are disabled and why (property status, when it is okay, it is enabled, otherwise disabled). The difficult part to me seems to be using of aliases. Somewhere, it refers to i2c_csi_dsi, elsewhere it is just i2c3...

Can you post these outputs?

$ ls /sys/class/i2c-adapter/i2c-*
$ ls /sys/class/i2c-adapter/i2c-*/of_node

The appropriate DTS sources for bookworm should be located here: https://elixir.bootlin.com/linux/v6.1/source/arch/arm/boot/dts. The of_node can help to find out where it refers into the DTS. It is a bit of mess but this is probably the way to figure out, what is going on. I could find multiple i2c controllers at https://elixir.bootlin.com/linux/v6.1/source/arch/arm/boot/dts/bcm2711.dtsi#L214.

It is about finding out, what is enabled in the /boot/config.txt, how does it refer into the DTS and thus to learn how to configure it. And probably, my notes in that commit might be incomplete or misleading and thus I'd like to make sure it is correct.

@3dti
Copy link

3dti commented May 5, 2024

I noticed that my config.txt had dtoverlay=i2c_arm=on, and your has i2c_vc=on...
After Googling the difference, I tried changing to VC instead of ARM, but since rebooting the CM4 hasn't come back online. Its running at my office. I will post the results of your 2 commands tomorrow! Thank you so much. This board would be perfect for us, if we can handle the INA!

@3dti
Copy link

3dti commented May 5, 2024

I noticed that my config.txt had dtoverlay=i2c_arm=on, and your has i2c_vc=on... After Googling the difference, I tried changing to VC instead of ARM, but since rebooting the CM4 hasn't come back online. Its running at my office. I will post the results of your 2 commands tomorrow! Thank you so much. This board would be perfect for us, if we can handle the INA!

It finally did come online... AND IT DID THE TRICK!!! With _vc instead of _arm, I now have:
/dev/i2c-0 /dev/i2c-10 /dev/i2c-20 /dev/i2c-21 /dev/i2c-22

... and 0x43 is present on bus 10! And my python script can read it!

$ i2cdetect -y 10
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
00:                         -- -- -- -- 0c -- -- --
10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 2f
30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
40: -- -- -- 43 -- -- -- -- -- -- -- -- -- -- -- --
50: -- 51 -- -- -- -- -- -- -- -- -- -- -- -- -- --
60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
70: -- -- -- -- -- -- -- --

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!!!

@jviki
Copy link
Contributor Author

jviki commented May 5, 2024

I noticed that my config.txt had dtoverlay=i2c_arm=on, and your has i2c_vc=on... After Googling the difference, I tried changing to VC instead of ARM, but since rebooting the CM4 hasn't come back online. Its running at my office. I will post the results of your 2 commands tomorrow! Thank you so much. This board would be perfect for us, if we can handle the INA!

It finally did come online... AND IT DID THE TRICK!!! With _vc instead of _arm, I now have: /dev/i2c-0 /dev/i2c-10 /dev/i2c-20 /dev/i2c-21 /dev/i2c-22

... and 0x43 is present on bus 10! And my python script can read it! $ i2cdetect -y 10 0 1 2 3 4 5 6 7 8 9 a b c d e f 00: -- -- -- -- 0c -- -- -- 10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 2f 30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 40: -- -- -- 43 -- -- -- -- -- -- -- -- -- -- -- -- 50: -- 51 -- -- -- -- -- -- -- -- -- -- -- -- -- -- 60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 70: -- -- -- -- -- -- -- --

Thanks a million. Haven't completely understood the difference between _vc and _arm, but comparing to your config.txt made the difference!

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.

Thanks a million!!!

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.

@3dti
Copy link

3dti commented May 5, 2024

I noticed that my config.txt had dtoverlay=i2c_arm=on, and your has i2c_vc=on... After Googling the difference, I tried changing to VC instead of ARM, but since rebooting the CM4 hasn't come back online. Its running at my office. I will post the results of your 2 commands tomorrow! Thank you so much. This board would be perfect for us, if we can handle the INA!

It finally did come online... AND IT DID THE TRICK!!! With _vc instead of _arm, I now have: /dev/i2c-0 /dev/i2c-10 /dev/i2c-20 /dev/i2c-21 /dev/i2c-22
... and 0x43 is present on bus 10! And my python script can read it! $ i2cdetect -y 10 0 1 2 3 4 5 6 7 8 9 a b c d e f 00: -- -- -- -- 0c -- -- -- 10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 2f 30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 40: -- -- -- 43 -- -- -- -- -- -- -- -- -- -- -- -- 50: -- 51 -- -- -- -- -- -- -- -- -- -- -- -- -- -- 60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 70: -- -- -- -- -- -- -- --
Thanks a million. Haven't completely understood the difference between _vc and _arm, but comparing to your config.txt made the difference!

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.

Thanks a million!!!

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...
You are totally right, the RTC section does mention the correct overlay... Maybe Waveshare expects you to run through ALL settings, in the order in which they are mentioned on their wiki?!? Anyway, MEGA delighted you helped me sort this!

@jviki jviki force-pushed the hwmon-ina219 branch 3 times, most recently from 4c21ceb to 2611ee9 Compare May 6, 2024 21:22
@AppVeyorBot
Copy link

Build nut 2.8.2.1715-master failed (commit 9da73e69dc by @jviki)

@jviki jviki force-pushed the hwmon-ina219 branch 2 times, most recently from 23cba55 to 14adcd9 Compare May 6, 2024 21:46
@jviki
Copy link
Contributor Author

jviki commented May 6, 2024

Quite good as noted before. Got a few comments and proposed changes while reading the code. Also, can you please

Thanks for review. I'll address the comments soon.

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).

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.

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).

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.

Also, a NEWS.adoc line to welcome the new driver would be useful :)

👍

I moved part of the commit message into docs/man/hwmon_ina219.txt. It checked the generated man page and it seems to look good.

@jviki
Copy link
Contributor Author

jviki commented May 6, 2024

Would you have a data dump (e.g. an upsc report) to stash for comparisons later?

I am not sure what exactly do you mean. Simple the output?

battery.charge: 100
battery.charge.high: 75
battery.charge.low: 15
battery.current: 0.000
battery.voltage: 4.204
device.description: Bidirectional Current/Power Monitor With I2C Interface
device.mfr: Texas Instruments
device.model: INA219
device.type: ups
driver.debug: 0
driver.flag.allow_killpower: 0
driver.name: hwmon_ina219
driver.parameter.pollinterval: 2
driver.parameter.port: /dev/null
driver.parameter.synchronous: auto
driver.state: quiet
driver.version: 2.8.1.1
driver.version.internal: 0.1
ups.mfr: Texas Instruments
ups.model: INA219
ups.status: OL HB
ups.type: ups

Now when default.battery.voltage.nominal = 3.6 is put into /etc/ups.conf into the driver's section:

battery.charge: 100
battery.charge.high: 75
battery.charge.low: 15
battery.current: -0.125
battery.voltage: 4.228
battery.voltage.nominal: 3.6
device.description: Bidirectional Current/Power Monitor With I2C Interface
device.mfr: Texas Instruments
device.model: INA219
device.type: ups
driver.debug: 0
driver.flag.allow_killpower: 0
driver.name: hwmon_ina219
driver.parameter.default.battery.voltage.nominal: 3.6
driver.parameter.pollinterval: 2
driver.parameter.port: auto
driver.parameter.synchronous: auto
driver.state: quiet
driver.version: 2.8.1.1
driver.version.internal: 0.01
ups.mfr: Texas Instruments
ups.model: INA219
ups.status: OL CHRG HB
ups.type: ups

or when the particular low and high are given:

battery.charge: 100
battery.charge.high: 75
battery.charge.low: 15
battery.current: -0.130
battery.voltage: 4.228
battery.voltage.high: 4.25
battery.voltage.low: 3
device.description: Bidirectional Current/Power Monitor With I2C Interface
device.mfr: Texas Instruments
device.model: INA219
device.type: ups
driver.debug: 0
driver.flag.allow_killpower: 0
driver.flag.default.: enabled
driver.name: hwmon_ina219
driver.parameter.default.battery.voltage.high: 4.25
driver.parameter.default.battery.voltage.low: 3
driver.parameter.pollinterval: 2
driver.parameter.port: auto
driver.parameter.synchronous: auto
driver.state: quiet
driver.version: 2.8.1.1
driver.version.internal: 0.01
ups.mfr: Texas Instruments
ups.model: INA219
ups.status: OL CHRG HB
ups.type: ups

@jviki
Copy link
Contributor Author

jviki commented May 7, 2024

@jimklimov How can I fix the spell checker issues?

drivers/hwmon_ina219.c Outdated Show resolved Hide resolved
drivers/hwmon_ina219.c Outdated Show resolved Hide resolved
drivers/hwmon_ina219.c Outdated Show resolved Hide resolved
Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
@jviki jviki force-pushed the hwmon-ina219 branch 3 times, most recently from 904be11 to 576c345 Compare May 13, 2024 19:59
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>
@jviki
Copy link
Contributor Author

jviki commented May 13, 2024

Current upsc output:

$ upsc test
Init SSL without certificate database
battery.charge: 91
battery.charge.low: 15
battery.current: 0.000
battery.voltage: 4.144
battery.voltage.nominal: 3.6
device.description: Bidirectional Current/Power Monitor With I2C Interface
device.mfr: Texas Instruments
device.model: INA219
device.type: ups
driver.debug: 0
driver.flag.allow_killpower: 0
driver.name: hwmon_ina219
driver.parameter.default.battery.voltage.nominal: 3.6
driver.parameter.pollinterval: 2
driver.parameter.port: auto
driver.parameter.synchronous: auto
driver.state: quiet
driver.version: 2.8.1.1
driver.version.internal: 0.01
ups.mfr: Texas Instruments
ups.model: INA219
ups.status: OL
ups.type: ups

@jimklimov
Copy link
Member

Thanks a lot for the changes, will read into them tomorrow :)
For now a quick question: zero readings for current - are they correct (also what the sysfs node reports) or some parsing problem?

@jviki
Copy link
Contributor Author

jviki commented May 14, 2024

Thanks a lot for the changes, will read into them tomorrow :) For now a quick question: zero readings for current - are they correct (also what the sysfs node reports) or some parsing problem?

Is this listing more satisfying? 😆 (I unplugged the power in and it now runs on battery only, see DISCHRG flag.)

$ upsc test
Init SSL without certificate database
battery.charge: 87
battery.charge.low: 15
battery.current: 0.081
battery.voltage: 4.092
battery.voltage.nominal: 3.6
device.description: Bidirectional Current/Power Monitor With I2C Interface
device.mfr: Texas Instruments
device.model: INA219
device.type: ups
driver.debug: 0
driver.flag.allow_killpower: 0
driver.name: hwmon_ina219
driver.parameter.default.battery.voltage.nominal: 3.6
driver.parameter.pollinterval: 2
driver.parameter.port: auto
driver.parameter.synchronous: auto
driver.state: quiet
driver.version: 2.8.1.1
driver.version.internal: 0.01
ups.mfr: Texas Instruments
ups.model: INA219
ups.status: OB DISCHRG
ups.type: ups

…cription [networkupstools#2430]

Clarified existing pre-set combos recognized by the driver code.

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
@jimklimov
Copy link
Member

Hm, duplicate sysf(s)_dir in usage is not needed - vars/flags are auto-documented, I will drop the extra:

:; ./drivers/hwmon_ina219 -h | grep sys
Path to sysfs dir of hwmon (/sys/class/hwmon) : -x sysfs_dir=<value>
  sysf_dir = /sys/class/hwmon (sysfs hwmon directory)

… 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>
@desertwitch
Copy link
Contributor

This seems really cool and innovative - just wanted to say thanks a lot for the efforts to everyone involved. 💯
Looking forward to experimenting with this down the road, just need to get some hardware parts first...

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…ls#2430]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
@jimklimov
Copy link
Member

Synced with current codebase to minimize surprises with upcoming merge.

@jimklimov jimklimov added the ready / gonna merge The PR is in final cycles leading to merge unless someone logs an objection before we hit the button label May 14, 2024
jimklimov added a commit to jimklimov/nut that referenced this pull request May 14, 2024
…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>
jimklimov added a commit to jimklimov/nut that referenced this pull request May 14, 2024
…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>
@jimklimov jimklimov merged commit ded86cb into networkupstools:master May 15, 2024
27 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Linux Some issues are specific to Linux as a platform ready / gonna merge The PR is in final cycles leading to merge unless someone logs an objection before we hit the button
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

None yet

5 participants