calculation in C for C2000 device

Question

I'm having some trouble with my C code. I have an ADC which will be used to determine whether to shut down (trip zone) the PWM which I'm using. But my calculation seem to not work as intended, because the ADC shuts down the PWM at the wrong voltage levels. I initiate my variables as:

float32 current = 5;
Uint16 shutdown = 0;

and then I calculate as:

// Save the ADC input to variable
adc_info->adc_result0 = AdcRegs.ADCRESULT0>>4;              //bit shift 4 steps because adcresult0 is effectively a 12-bit register not 16-bit, ADCRESULT0 defined as Uint16

current = -3.462*((adc_info->adc_result0/1365) - 2.8);

// Evaluate if too high or too low
if(current > 9 || current < 1)
{
    shutdown = 1;
}
else
{
    shutdown = 0;
}

after which I use this if statement:

if(shutdown == 1)
{
    EALLOW;                                                 // EALLOW protected register
    EPwm1Regs.TZFRC.bit.OST = 1;                // Force a one-shot trip-zone interrupt
    EDIS;                                                   // end write to EALLOW protected register
}

So I want to trip the PWM if current is above 9 or below 1 which should coincide with an adc result of <273 (0x111) and >3428 (0xD64) respectively. The ADC values correspond to the voltages 0.2V and 2.51V respectively. The ADC measure with a 12-bit accuracy between the voltages 0 and 3V.

However, this is not the case. Instead, the trip zone goes off at approximately 1V and 2.97V. So what am I doing wrong?

Why don't you use `adc_info->adc_result0` directly? `if ((273.0 <= adc_info->adc_result0) && (adc_info->adc_result0 <= 3428.0)) shutdown = 0; else shutdown = 1;` — pmg, Apr 09 '14 at 11:10
`if current is above 9 or below 1 which` where are units? `Amps` or `milli amps` or what? — gangadhars, Apr 09 '14 at 11:14
pmg - it's just so it's easier to set the limits if I want to change them. The samples don't tell me anything without converting them into amps first. SGG - The unit is amps. 1/1365 is V/sample and -3.462 is A/V — user3514815, Apr 09 '14 at 11:25

score 4 · Answer 1 · edited May 23 '17 at 10:25

4

adc_info->adc_result0/1365

Did you do integer division here while assuming float?

Try this fix:

adc_info->adc_result0/1365.0

Also, the @pmg's suggestion is good. Why spending cycles on calculating the voltage, when you can compare the ADC value immediately against the known bounds?

if (adc_info->adc_result0 < 273 || adc_info->adc_result0 > 3428)
{
    shutdown = 1;
}
else
{
    shutdown = 0;
}

If you don't want to hardcode the calculated bounds (which is totally understandable), then define them as calculated from values which you'd want to hardcode literally:

#define VOLTAGE_MIN 0.2
#define VOLTAGE_MAX 2.51
#define AREF 3.0
#define ADC_PER_VOLT (4096 / AREF)

#define ADC_MIN (VOLTAGE_MIN * ADC_PER_VOLT) /* 273 */
#define ADC_MAX (VOLTAGE_MAX * ADC_PER_VOLT) /* 3427 */

/* ... */
    shutdown = (adcresult < ADC_MIN || adcresult > ADC_MAX) ? 1 : 0;
/* ... */

When you've made sure to grasp the integer division rules in C, consider a little addition to your code style: to always write constant coefficients and divisors with a decimal (to make sure they get a floating point type), e.g. 10.0 instead of 10 — unless you specifically mean integer division with truncation. Sometimes it may also be a good idea to specify float literal precision with appropriate suffix.

edited May 23 '17 at 10:25

Community

1
1

answered Apr 09 '14 at 11:14

ulidtko

14,740
10
56
88

adc_result0 is defined as a Uint32 in the struct. – user3514815 Apr 09 '14 at 11:26
So I am right. You do integer division, which truncates the fractional part of result. Force it to float by making `1365` a float constant rather then integer. – ulidtko Apr 09 '14 at 11:27
It works now that I use the bounds directly. But it still doesn't explain what I did wrong in my calculation. I've done the exact same calculation in matlab and got the result I expected. – user3514815 Apr 09 '14 at 11:31
I tried switching 1365 to 1365.0 but it didn't seem to fix the problem. – user3514815 Apr 09 '14 at 11:33
Matlab uses different rules for integer division. In C, `9 / 2 == 4` but `9 / 2.0 == 4.5`. – ulidtko Apr 09 '14 at 11:40
I'm going to use a matlab script to calculate the adc_result0 limits from the current instead, as you suggested. If you have a suggestion on what I should have changed to make my original code run as it's supposed to I would still like to know though. Always good to learn. – user3514815 Apr 09 '14 at 11:40
3

Since the intended type is `float32`, use _float_ literals! Do not use double literals, do not use int literals. `VOLTAGE_MIN 0.2f`, `4096f / AREF` and so on. – Lundin Apr 09 '14 at 11:52
@Lundin you are correct, but that's a secondary remark comparing to the other issues with this code. I also didn't want to bring in yet another C-specific complication. – ulidtko Apr 09 '14 at 12:20
Thanks so much for your help! I'm going to stick to comparing the adc values directly but next time I need to actually calculate something I will keep this in mind. Thanks again. – user3514815 Apr 09 '14 at 12:21
@ulidtko Indeed, but the original problem was mixing int, float and double pretty much randomly. – Lundin Apr 09 '14 at 12:33
No, the original problem was unnoticed truncation during integer division. – ulidtko Apr 09 '14 at 12:43
@ulidtko: It is true that integer division was the cause of the *specific* problem in question, however the general bad practice of poor type agreement is the root of the problem. If user3514815 codes like that habitually, many other related bugs are likely to occur. – Clifford Apr 09 '14 at 18:25
@Clifford I could perhaps object to this. *(The most useful solution lies in the language design plane (since this is a language issue, and tightening your code style is only going to be an eternal workaround of that issue) — and seems to be in the form of overloaded literals a-la Haskell's `Num`.)* In the end, that's not a kind of discussion I'd want to bring into the answer. – ulidtko Apr 09 '14 at 22:49
1

Just so I don't make the same mistake again, would my statement have been ok as long as I specify that the other constants are the same data type as the variable I'm sending to? I guess it was ok to use my Uint32 (that's a long, btw) variable when calculating a float but the constants I use need to be floats as well? Or did I misunderstand? – user3514815 Apr 10 '14 at 06:06
@ulidtko: Fair enough (though you lost me at the part on italics!), my comment was for the general benefit of the reader and not a criticism of your answer. If I thought I had a significantly better answer I'd have posted one. – Clifford Apr 10 '14 at 10:36
@user3514815: In general; prefer explicit type agreement, over explicit type casts, over implicit type casts. But specifically your problem is solved by either of the solutions in this answer. The second *integer only* solution being better floating point operations on an embedded system with no FPU are slow and pull in a lot of code, and are seldom necessary. An expression with a floating-point operand is explicitly promoted to floating-point, so making teh constant floating point makes the whole expression floating point. For strong type agreement `1365.0f` should be used over `1365.0`. – Clifford Apr 10 '14 at 10:47
@user3514815, no. To avoid repeating the same mistake, remember that `9 / 2 = 4` in C. When dividing integer by integer in C, you get back integer. – ulidtko Apr 10 '14 at 13:00

calculation in C for C2000 device

1 Answers1