Changed input current limit comparison values from double to float and fixed 900mA setting

[rsingletonDS]

There are two issues being addressed here.

1. Setting the input current limit to 0.9 A didn't work because the comparison was against CURRENT_LIM_900 (a register setting) rather than against 0.9.
2. Once I fixed that, it was setting the limit to 0.5A even though I passed in 0.9. This was because the comparisons were comparing floats and doubles. Changing the constants from doubles to floats fixed the problem.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

---

Rob Singleton seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[github-actions]

Memory usage change @ ea5bdda

Board	flash	%	RAM for global variables	%
arduino:mbed_nano:nano33ble	💚 -64 - 0	-0.01 - 0.0	0 - 0	0.0 - 0.0
arduino:mbed_nano:nanorp2040connect	💚 -24 - 0	-0.0 - 0.0	0 - 0	0.0 - 0.0
arduino:mbed_portenta:envie_m7:target_core=cm4	N/A	N/A	N/A	N/A
arduino:mbed_portenta:envie_m7	N/A	N/A	N/A	N/A
arduino:samd:mkrgsm1400	💚 -24 - 0	-0.01 - 0.0	0 - 0	0.0 - 0.0
arduino:samd:mkrnb1500	💚 -24 - 0	-0.01 - 0.0	0 - 0	0.0 - 0.0
arduino:samd:mkrvidor4000	💚 -24 - 0	-0.01 - 0.0	0 - 0	0.0 - 0.0
arduino:samd:mkrwifi1010	💚 -24 - 0	-0.01 - 0.0	0 - 0	0.0 - 0.0
arduino:samd:nano_33_iot	💚 -24 - 0	-0.01 - 0.0	0 - 0	0.0 - 0.0

Click for full report table

Board	examples/BatteryCharger flash	%	examples/BatteryCharger RAM for global variables	%	examples/BatteryChargerInterrupt flash	%	examples/BatteryChargerInterrupt RAM for global variables	%	examples/PMICBoostMode flash	%	examples/PMICBoostMode RAM for global variables	%	examples/PMICFaultCheck flash	%	examples/PMICFaultCheck RAM for global variables	%
arduino:mbed_nano:nano33ble	0	0.0	0	0.0	-64	-0.01	0	0.0	0	0.0	0	0.0	0	0.0	0	0.0
arduino:mbed_nano:nanorp2040connect	-24	-0.0	0	0.0	-24	-0.0	0	0.0	0	0.0	0	0.0	0	0.0	0	0.0
arduino:mbed_portenta:envie_m7:target_core=cm4	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A
arduino:mbed_portenta:envie_m7	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A
arduino:samd:mkrgsm1400	-24	-0.01	0	0.0	-24	-0.01	0	0.0	0	0.0	0	0.0	0	0.0	0	0.0
arduino:samd:mkrnb1500	-24	-0.01	0	0.0	-24	-0.01	0	0.0	0	0.0	0	0.0	0	0.0	0	0.0
arduino:samd:mkrvidor4000	-24	-0.01	0	0.0	-24	-0.01	0	0.0	0	0.0	0	0.0	0	0.0	0	0.0
arduino:samd:mkrwifi1010	-24	-0.01	0	0.0	-24	-0.01	0	0.0	0	0.0	0	0.0	0	0.0	0	0.0
arduino:samd:nano_33_iot	-24	-0.01	0	0.0	-24	-0.01	0	0.0	0	0.0	0	0.0	0	0.0	0	0.0

Click for full report CSV

Board,examples/BatteryCharger<br>flash,%,examples/BatteryCharger<br>RAM for global variables,%,examples/BatteryChargerInterrupt<br>flash,%,examples/BatteryChargerInterrupt<br>RAM for global variables,%,examples/PMICBoostMode<br>flash,%,examples/PMICBoostMode<br>RAM for global variables,%,examples/PMICFaultCheck<br>flash,%,examples/PMICFaultCheck<br>RAM for global variables,%
arduino:mbed_nano:nano33ble,0,0.0,0,0.0,-64,-0.01,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0
arduino:mbed_nano:nanorp2040connect,-24,-0.0,0,0.0,-24,-0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0
arduino:mbed_portenta:envie_m7:target_core=cm4,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A
arduino:mbed_portenta:envie_m7,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A
arduino:samd:mkrgsm1400,-24,-0.01,0,0.0,-24,-0.01,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0
arduino:samd:mkrnb1500,-24,-0.01,0,0.0,-24,-0.01,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0
arduino:samd:mkrvidor4000,-24,-0.01,0,0.0,-24,-0.01,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0
arduino:samd:mkrwifi1010,-24,-0.01,0,0.0,-24,-0.01,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0
arduino:samd:nano_33_iot,-24,-0.01,0,0.0,-24,-0.01,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0

[facchinm]

Hi @rsingletonDS ,
the CURRENT_LIM_900 fix is surely correct (thanks for spotting it) but I'm not convinced by the float ->double fix (that shouldn't be needed due to automatic promotion). Could you split the PR in two commits to tackle the different problems so we can merge them separately?

[rsingletonDS]

Martino, thanks for following up. Without the float specification I still won't be able to set 900 mA, so let me try to convince you.

I instrumented the function as follows:

    if (current >= 0.5) {
        Serial.printf("Setting current to %f\n", current);
        Serial.println("500");
        current_val = CURRENT_LIM_500;
    }
    if (current >= 0.9) {
        Serial.println("900");
        current_val = CURRENT_LIM_900;
    }
    if (current >= 1.2) {
        Serial.println("1.2");
        current_val = CURRENT_LIM_1200;
    }

This is all I get. It does not set the current to 900 mA as I require.

Setting current to 0.900000
500

My first fix was to change the constants to 1 mV lower:

    if (current >= 0.899) {
        Serial.println("900");
        current_val = CURRENT_LIM_900;
    }

This fixed it:

Setting current to 0.900000
500
900

But during internal code review, one of my co-workers pointed out that the constant was probably a double while current is a float. He suggested making the constants floats:

    if (current >= 0.900f) {
        Serial.println("900");
        current_val = CURRENT_LIM_900;
    }

That also fixed it:

Setting current to 0.900000
500
900

Since that was the better solution, that's what I submitted in the pull request. Please reconsider this change.

Regards,
Rob

[per1234]

the CURRENT_LIM_900 fix is surely correct (thanks for spotting it) but I'm not convinced by the float ->double fix (that shouldn't be needed due to automatic promotion). Could you split the PR in two commits to tackle the different problems so we can merge them separately?

@rsingletonDS has now created a dedicated PR for the CURRENT_LIM_900 fix: #35