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

[Bug]: ina3221 inverts current over certain values (maybe signed int overflow?) #5933

Open
sgtwilko opened this issue Jan 25, 2025 · 15 comments · May be fixed by KodinLanewave/INA3221#9
Open
Labels
bug Something isn't working

Comments

@sgtwilko
Copy link

Category

Other

Hardware

Rak4631

Firmware Version

2.4.* to at least 2.5.15

Description

I have an ina3221 setup with input 1 showing power being used, input 2 showing solar being provided, and input 3 showing the actual current flowing in out of the battery.

The first two should always be positive, the third should be positive when drawing current from the battery and negative when it's being charged.

With low current this is always the case, when one of them goes over about 250mA it inverts that value.

I suspect this is a mismatch in int lengths between the library that's taking the values and where it's being stored as it's typical of a 2s compliment overflow (although strangely would seem to be 9 bits)

Relevant log output

@sgtwilko sgtwilko added the bug Something isn't working label Jan 25, 2025
@b8b8
Copy link

b8b8 commented Jan 25, 2025

Have you correctly changed the address of the INA3221 off the default so it is correctly recognized by the firmware?
You need to bridge SDA.

Image

@isseysandei
Copy link
Contributor

You beat me to it, I was going to open a report about this soon... Same issue as you, but after a while it fixes by itself, idk if happen the same to you.

Image

@sgtwilko
Copy link
Author

Have you correctly changed the address of the INA3221 off the default so it is correctly recognized by the firmware?
You need to bridge SDA.

Yes, you don't get any values at all if you haven't changed the address.

You can see from the images here in getting values, and that they suddenly reverse ( particularly channel 2, the solar input)

Screenshot_20250124-113035~2.png

Screenshot_20250125-135403~2.png

Screenshot_20250112-132048~2.png

This does seem very typical of a twos compliment overflow. Where large values cause the first bit of the word to become a 1 which is then interpreted as a negative value.

@sgtwilko
Copy link
Author

You beat me to it, I was going to open a report about this soon... Same issue as you, but after a while it fixes by itself, idk if happen the same to you.

From my limited observations, once it falls back below a threshold it reverts back to reporting correctly.

That seems slightly different to what we see in your graph.

@dblmca
Copy link

dblmca commented Jan 30, 2025

I sent the dev a note about this back in Nov.
I don't have the code in front of me, but the issue was something like shunt_uV becomes negative due to overflow the calculated current becomes incorrect.

The fix should be to use an int32_t in getshuntvolt

@fifieldt
Copy link
Contributor

Had a quick look in the code and there is indeed a uint16 in there at the moment.

@sgtwilko
Copy link
Author

I sent the dev a note about this back in Nov.
I don't have the code in front of me, but the issue was something like shunt_uV becomes negative due to overflow the calculated current becomes incorrect.

The fix should be to use an int32_t in getshuntvolt

I also had a look, before seeing your comment.

It's absolutely in need of an int32_t as the multiplication by 40 overflows the int16.

Going to try this on my local and see if it works

@sgtwilko
Copy link
Author

sgtwilko commented Feb 2, 2025

I've only just managed to try my change (been unwell), and for the first time I'm seeing higher values.

I've got about 40W of 12v solar which even in cloudy rainy Britain should generate more than 260mA, and it looks like it's working ok as the channel which monitors the power usage of the node is showing the same sort of value as before.

Image

Image

I've only just spotted that the code for the inna3221 code is from a different repo.
That repo is a fork of another which already seems to have some sort of fix for this issue.
I'll figure out where to PR

@sgtwilko
Copy link
Author

sgtwilko commented Feb 6, 2025

I've now managed to confirm the figure using a multimeter to monitor the current in line with one of the channels.

It is correctly reporting current up to an amp now.

I'll get the PR opened today.

@dblmca
Copy link

dblmca commented Feb 6, 2025

I've now managed to confirm the figure using a multimeter to monitor the current in line with one of the channels.

It is correctly reporting current up to an amp now.

I'll get the PR opened today.

Thank you for staying on top of this, I've been meaning to check how I did it back in Nov, but... life. Will be nice not to have to worry about one more library during builds.
Cheers!

@dblmca
Copy link

dblmca commented Feb 7, 2025

has anyone been able to check if the INA3221 still works in .19, .20 or .21?
I just tried and it didn't seem to work, but I'm running wonky custom hardware, and it could just be me.

@sgtwilko
Copy link
Author

sgtwilko commented Feb 7, 2025

I'm running a custom built .20,
It works for power telemetry, but I've been having issues getting it to be used for battery %.

@isseysandei
Copy link
Contributor

has anyone been able to check if the INA3221 still works in .19, .20 or .21? I just tried and it didn't seem to work, but I'm running wonky custom hardware, and it could just be me.

I flashed .21 on my NRF nodes and I noticed that Battery % from either INA3221 and 219 is broken. Also, idk if it related, but the INA219 values are no longer passed by ch3 curr/voltage. It stops broadcasting that packet, no problem with the INA3221...

@sgtwilko
Copy link
Author

sgtwilko commented Feb 7, 2025

Sounds like a different issue to this one, but worth comparing the source from an earlier version to .20
When did it work last?

@dblmca
Copy link

dblmca commented Feb 7, 2025

Sounds like a different issue to this one, but worth comparing the source from an earlier version to .20 When did it work last?

I went back to the .18 code base and its working fine.
Ill take a look at the later ones this weekend to see what's up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants