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

normalize routine in setMaxCurrent not working? #11

Closed
morrowwm opened this issue Feb 1, 2022 · 18 comments · Fixed by #12
Closed

normalize routine in setMaxCurrent not working? #11

morrowwm opened this issue Feb 1, 2022 · 18 comments · Fixed by #12
Assignees
Labels
bug Something isn't working

Comments

@morrowwm
Copy link

morrowwm commented Feb 1, 2022

I am setting the calibration register using

INA.setMaxCurrentShunt(5, 0.097563559322034, false);

added to setup() in your INA226_demo sketch and seeing values that match what my multimeter measures fairly closely.

I uncommented and augmented the debug print statements in setMaxCurrentShunt():

  // normalize the LSB to a round number
  // LSB will increase
  if (normalize)
  {
     Serial.print("Initial current_LSB:\t");
     Serial.println(_current_LSB, 10);
    uint32_t factor = 1;
    while (_current_LSB < 1)
    {
      _current_LSB *= 10;
      factor *= 10;
    }
    _current_LSB = 10.0 / factor;
     Serial.print("factor: "); Serial.println(factor); 
     Serial.print("current_LSB:\t");
     Serial.println(_current_LSB, 10);
  }

This showed:

Initial current_LSB:	0.0001525879
factor: 10000 current_LSB:	0.0010000001
Calibration:	34
Current_LSB:	0.0010000001192092 uA / bit

The _current_LSB value has shifted by a factor of 10

It looks like

_current_LSB = 10.0 / factor;

should be

_current_LSB = 1.0 / factor;

I made that change and see:

Initial current_LSB:	0.0001525879
factor: 10000
current_LSB:	0.0001000000
Calibration:	20D
Current_LSB:	0.0000999999904632 uA / bit

Thank you for this library, and figuring out the calibration process.

@RobTillaart
Copy link
Owner

Thanks for reporting the issue, might take a few days

@RobTillaart RobTillaart self-assigned this Feb 1, 2022
@RobTillaart RobTillaart added the bug Something isn't working label Feb 1, 2022
@RobTillaart
Copy link
Owner

Will start investigating today.

@RobTillaart
Copy link
Owner

No setup to test. Your analysis is looks correct.

RobTillaart added a commit that referenced this issue Feb 2, 2022
@RobTillaart
Copy link
Owner

@morrowwm
Created a PR - see develop branch. Can you check this branch?

RobTillaart added a commit that referenced this issue Feb 2, 2022
RobTillaart added a commit that referenced this issue Feb 2, 2022
@morrowwm
Copy link
Author

morrowwm commented Feb 2, 2022

It looks good. The current calculated by the INS226 is very close to that measured by my multimeter.

@morrowwm
Copy link
Author

morrowwm commented Feb 2, 2022

Alternatively, what do you think of this approach to forcing current_LSB to be an integer, i.e. normalized?
INA226.cpp.txt

Starting at line 217, calculate calib, forcing no fractional part with round(). Then recalculate current_LSB using that calib.

This approach does not change the max current allowed as much.

Initial current_LSB:	0.00015259
Normalized current_LSB:	0.00015255 uA / bit
Max current:	5.00 A

@RobTillaart
Copy link
Owner

Then recalculate current_LSB using that calib.

why didn't I think of that!
Very good point as that way the internal variables stay way more consistent.

forcing current_LSB to be an integer

Would make sense to me if one could do the whole math in integer units.
Can you name some advantages?
I make a note in the future section in the readme,

@morrowwm
Copy link
Author

morrowwm commented Feb 2, 2022 via email

RobTillaart added a commit that referenced this issue Feb 2, 2022
@RobTillaart
Copy link
Owner

Just pushed a new version of .cpp file.

I added some #ifdef at the start of the function for switching on/off print statements.
furthermore it includes your suggestion to calculate current_LSB back from rounded calib.

give it a try.

@RobTillaart
Copy link
Owner

Doing all the math in integer will need more testing and I have so HW nearby (and enough time)
Note in readme to keep the thought

RobTillaart added a commit that referenced this issue Feb 2, 2022
@morrowwm
Copy link
Author

morrowwm commented Feb 2, 2022

give it a try.

I had to remove the old normalize lines, starting at about line 235. Then I defined printdebug and also added more digits to the _maxCurrent debug:

With normalize = false:

normalize:	 false
initial current_LSB:	0.00015259 uA / bit
factor:	1
Final current_LSB:	0.00015259 uA / bit
Calibration:	344
Max current:	5.00000000 A
Shunt:	0.09756356 ohm

BUS	SHUNT	CURRENT	POWER
6.984	-4.600	-47.150	331.879
6.986	-4.600	-47.150	331.879
6.986	-4.600	-47.150	331.879

And with normalize = true:

normalize:	 true
initial current_LSB:	0.00015259 uA / bit
Prescale current_LSB:	0.00015255 uA / bit
factor:	1
Final current_LSB:	0.00015255 uA / bit
Calibration:	344
Max current:	4.99889278 A <--- no longer 5.0, but an integer multiple of current_LSB
Shunt:	0.09756356 ohm

BUS	SHUNT	CURRENT	POWER
6.986	-4.603	-47.139	331.805
6.985	-4.600	-47.139	331.805

@morrowwm
Copy link
Author

morrowwm commented Feb 2, 2022 via email

@RobTillaart
Copy link
Owner

So the new version works and still mayches your multimeter?

@morrowwm
Copy link
Author

morrowwm commented Feb 2, 2022

So the new version works and still matches your multimeter?

Yes, it works well. Matches my multimeter to about 1 part in 400, both normalized and not.

I'll see if I can find a setup that will hold a stable, higher current and get more than three digits to measure.

@RobTillaart
Copy link
Owner

You might need to check the datasheet if you can get much better than 1 in 400 in terms of precission.

@morrowwm
Copy link
Author

morrowwm commented Feb 2, 2022 via email

@RobTillaart
Copy link
Owner

You might need to check the datasheet if you can get much better than 1 in 400 in terms of precision.

The datasheet mentions several causes of small errors (see below) which in the end add up so a difference of 0.25% is not bad at all, given that most multimeters are in the 0.2% range themselves. You need a DMM with 0.02% or better to make better measurements (professional or lab equipment)

image

@RobTillaart RobTillaart linked a pull request Feb 5, 2022 that will close this issue
@RobTillaart
Copy link
Owner

@morrowwm
Are there other (serious) bugs that must be fixed?
Otherwise I merge this version and start a new branch for other issues if they appear.

RobTillaart added a commit that referenced this issue Feb 8, 2022
* fix #11 normalize routine
* fix #13 sign handling
* add releaseNotes.md
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.

2 participants