ole00 / afterburner

GAL chip programmer for Arduino
166 stars 46 forks source link

Multiple bugs in Afterburner software (Calibration Offset) #50

Closed hubertushirsch closed 9 months ago

hubertushirsch commented 9 months ago

There are bugs in the Arduino sketch and in the PC software for this function. Both must be fixed together. I made a fork on February 2nd, 2024 to test the fix. https://github.com/hubertushirsch/afterburner // the unchanged fork of the master branch https://github.com/hubertushirsch/afterburner/tree/hubertushirsch-afterburner-B9-fix

afterburner.ino The offset range -20 to 25 (-co X) is transposed by the PC software into the range 0..9 and transmitted to the Arduino as command B0..B9. In the Arduino, however, the value 9 is not in the evaluated range, so the calibration around 0.25V can never take place.

You can see it when you send B9 to Afterburner on the serial terminal.

AFTerburner v.0.5.5
 varVpp
type 'h' for help
I: VPP calib. OK
>
B9
ER: cal offset failed
>

In addition, there is a typo in the explanation of the values 1:-1.5V ==> -0.15V

--- afterburner.ino     2024-01-28 22:29:17.000000000 +0100
+++ afterburner_fixed.ino       2024-01-29 21:12:33.365225100 +0100
@@ -2700,8 +2700,8 @@
       // small differences in analog ref which is ~3.3 V derived from LDO.
       case COMMAND_CALIBRATION_OFFSET: {
         int8_t offset = line[1] - '0';
-        if (offset >=0 && offset < 9) {
-          //0:-0.2V 1:-1.5V  2: -0.1V 3: -0.05V 4: 0V  5: 0.05V  6: 0.1V 7: 0.15V 8: 0.20V 9:0.25V
+        if (offset >=0 && offset <= 9) {
+          //0:-0.2V 1:-0.15V  2: -0.1V 3: -0.05V 4: 0V  5: 0.05V  6: 0.1V 7: 0.15V 8: 0.20V 9:0.25V
           calOffset = (offset - 4) * 5;
           Serial.print(F("Using cal offset: "));
           Serial.println(calOffset);

afterburner.c If this bug is fixed, a bug in the PC program afterburner.c still needs to be fixed. When the PC software is started, the global variable calOffset is initialized.

125: int calOffset = 0xFFFF; //calibration offset is not applied

What does 0xFFFF mean? This is compiler dependent. In an architecture where int is set to 16-bit, this would be -1, which is a valid calibration offset value in the range -20..25. Probably nobody has this architecture anymore today. In a 32- and 64-bit PC architecture compiler, int is set to be at least 32 bits, so 0xFFFF is a large positive number (65535). This is (for now) outside the valid offset range -20..25.

But what does the parsing of the command line do in afterburner.c? If there is a -co option on the command line, the value is read. If it is outside -20...25, a warning message appears.

afterburner.c lines 270..276
    } else if (strcmp("-co", param) == 0) {
         i++;
         calOffset = atoi(argv[i]);
         if (calOffset < -20 || calOffset > 25) {
             printf("Calibration offset out of range (-20..25 inclusive).\n");
         }
     }

Some lines later the values are corrected if they were outside the range.

afterburner.c lines 281..285
     if (calOffset < -20) {
         calOffset = -20;
     } else if (calOffset > 25) {
         calOffset = 25;
     }

However, this occurs outside the -co condition, so that calOffset then has a valid value in any case. If the -co option is missing, this corrects 0xFFFF (which is greater than 25) to 25.

What is the consequence: With every b command from the PC software to the Arduino, a calibration offset is transmitted, regardless of whether a -co option is specified or not. Without -co option 0xFFFF => 25 => B9 is transmitted. A Verbose output confirms this.

$ ./afterburner.exe -d COM15 -v b <=== Executing without -co xx
Afterburner v.0.5.5
opening serial: \\.\COM15
variable VPP board detected
read: 5 ''
opening serial: \\.\COM15
variable VPP board detected
sending 'B9' command...     <= B9 is sending
read: 28 'ER: cal offset failed'   <== This failed in unfixed Arduino software
ER: cal offset failed
sending 'b' command...
VPP voltages are scanned - this might take a while...
....

Just because the unfixed Arduino program ignores the value at B9, the error in the PC program has no effect. If afterburner.ino is already fixed, the calibration offset is set to 0.25V instead. See my fixed branch for changes in arduino.c.

As last: What should actually happen if the b command is called without -co?

Let's say I'm repairing the Afterburner hardware (pot or something) and need to recalibrate according to the instructions. Step 1: R9 in middle position Step 2: ./afterburner s Step 3: ./afterburner b What happens, there is an old offset in the Arduino EEPROM? I get incorrect values. Step 4: ./afterburner m ./afterburner b -co X

ole00 commented 9 months ago

Hi Hubert, thank you for the fixes. Well spotted. I noticed the 'ER: cal offset failed' error before and was going to look at it eventually. You fixed it already, so that's great.

What should actually happen if the b command is called without -co?

I agree, when the calibration offset is not set (-co is not passed as an argument) the calibration offset should be set to 0. That way it is more clear what is going on..

Thank you for your commits in your merge request, I'll merge it manually by using git (not by using the github's interface).

ole00 commented 9 months ago

Your code fixes have been merged to master. See the master branch. Thanks.

hubertushirsch commented 9 months ago

Thanks to you too. What I didn't consider: When I use an Arduino that has calibration data from a previous use in the EEPROM, the stored calibration offset is always used. If I do a calibration from the scratch,

./afterburner s

from the PC or the t command on the serial console, different values shown depending on whether the Arduino has already been used for Afterburner or is fresh out of the box.

hubertushirsch commented 9 months ago

I tested version 0.5.6. It works as expected.

Regarding the problem of complete recalibration when using the same Arduino. This may be necessary if changes or repairs have been made to the Afterburner circuit board.

I would then do the following (possibly include it in the setup instructions):

Pot R9 in the middle position ./afterburner b ; Clears the calibration offset (0.00V), ignore all command outputs. ./afterburner s ; Continue as in the original setup instructions

Note: Use b -co 0 on program version 0.5.5 or earlier to clear the calibration offset.