PIC16F EEPROM write errors

Question

I'm developing on a PIC16F1936 using the MPLAB X IDE with the XC8 complier. I've been given this project where I need to "upgrade" the software of a PCB such that it can interface with an external tool via an RS232 link.

I have the link working and I have also checked that the date being transmitted is correct. The problem that I am having is with a specific function which calibrates the setpoints for the PCB. I can't say too much on what the exact function of the PCB so sorry for not posting everything. Also, I have inherited this a lot of the code on this project, such as the code I'm about to show you.

So the calibration function works like this: it is called once the tool sends the relevant command to the PCB. After this (depending on command sent), the function will read the value of the summed value of 40 values from the ADC and saves this to the EEPROM. This is where it gets really messed up! The calibration works perfectly well when I have to ZERO the PCB, the problem arises when I have to set MAX. Oh! And the code is identical, apart from the EEPROM address...

Here is the code for calibration:

if (zero == 1)
{
    WR = 1;                                         // Set EEPROM_WRITE flag
    EEPROM_WRITE(0x05,(AN1_sum >> 0x08));           // load MSbyte of AN1_sum into NV memory location 1
    EEPROM_WRITE(0x06,(AN1_sum & 0xff));            // load LSbyte of AN1_sum into NV memory location 2
    EEPROM_WRITE(0x07,(AN4_sum >> 0x08));           // load MSbyte of AN4_sum into NV memory location 3
    EEPROM_WRITE(0x08,(AN4_sum & 0xff));            // load LSbyte of AN4_sum into NV memory location 4

    zero1 = AN1_sum;                                // update volatile memory for zero1
    zero4 = AN4_sum;                                // update volatile memory for zero4
    WR = 1;                                         //Set EEPROM_WRITE flag
    EEPROM_WRITE(0x09, 2);                          //Write 2 to the EEPORM

    while(WR){                                      //Loop till WR flag is reset by EEPROM CONTROLLER
        MAL = 1;                                        //Light MAL until WR is reset
    }
    for(KILLTIME = 0; KILLTIME <= 200000; KILLTIME++);
    RESET();
    for(KILLTIME = 0; KILLTIME <= 200000; KILLTIME++);
}
else if (MAX == 1)
{
    WR = 1;                                         //Set EEPROM_WRITE flag
    EEPROM_WRITE(0x01,(AN1_sum >> 0x08));           // load MSbyte of AN1_sum into NV memory location 5
    EEPROM_WRITE(0x02,(AN1_sum & 0xff));            // load LSbyte of AN1_sum into NV memory location 6
    WR = 1;                                         //Set EEPROM_WRITE flag
    EEPROM_WRITE(0x03,(AN4_sum >> 0x08));           // load MSbyte of AN4_sum into NV memory location 7
    EEPROM_WRITE(0x04,(AN4_sum & 0xff));            // load LSbyte of AN4_sum into NV memory location 8

    MAX1 = AN1_sum;                                 // update volatile memory for MAX1
    MAX4 = AN4_sum;                                 // update volatile memory for MAX4

    WR = 1;                                         //Set EEPROM_WRITE flag
    EEPROM_WRITE(0x9, 3);                           //Write 3 to the EEPROM

    while(WR){                                      //Loop till WR flag is reset by EEPROM CONTROLLER
        MAL = 1;                                        //Light MAL until WR is reset
    }
    for(KILLTIME = 0; KILLTIME <= 200000; KILLTIME++);
    RESET();
    for(KILLTIME = 0; KILLTIME <= 200000; KILLTIME++);
}

After the reset, the PCB restarts and goes through this initialisation process:

MAX1 = EEPROM_READ(0x01);                                               
MAX1 = (MAX1 << 8) + EEPROM_READ(0x02);
MAX4 = EEPROM_READ(0x03);                                 
MAX4 = (MAX4 << 8) + EEPROM_READ(0x04); 
zero1 = EEPROM_READ(0x05);                                           
zero1 = (zero1 << 8) + EEPROM_READ(0x06);                          
zero4 = EEPROM_READ(0x07);                                 
zero4 = (zero4 << 8) + EEPROM_READ(0x08); 

Simple enough right? This is only time in the code where MAX and zero are configured.

I have no idea of why this isn't working MAX should be whatever value AN1 and AN4 are at the time of execution. I've swapped the EEPROM addresses around and changed the name of the variables to make sure that MAX and ZERO are not being overwritten elsewhere in the code. No avail. MAX only wants to be the correct value when I say MAX1 = x and MAX 4 = y. It can't be something with the values of AN1 and AN4 as they work fine for TARE no matter what their value is.

Any ideas?

Just to add, I have tried the following solutions:

1. I swapped the EEPROM addresses such that MAX writes to addresses. I mean that MAX is now being written to 0x05-0x08 and ZERO is written to 0x01-0x05. Still I have the same problem. The value from the sensor is correctly written to ZERO but not MAX.

2. I've played around with the WR flag by placing more and removing them completely. NO CHANGE.

3. I've tried storing the values of AN1_sum and AN4_sum in a temporary variable as soon as the calibration function starts and used those variables instead.

4. I've tried reversing the roles of ZERO and MAX. By this, I mean that I set ZERO to the maximum setpoint and MAX to the minimum. I found that ZERO sets accurately and the value it has can affect the value of the MAX. For example, I first set applied 45 to both channels and then set the MAX. After the calibration MAX had a value of 26. I then applied 198 to both channels and set ZERO. ZERO held a final value of 198, while MAX changed to 48!

I've known for some time that MAX is the culprit but I do not know why or how this is happening. I've gone through the rest of the code and checked to see if MAX is being affected in any way but it isn't.

Answer 1

Try casting the second argument of EEPROM_WRITE() to the right type. EEPROM is arranged in 8bit memory locations and by the looks of it when doing (AN4_sum & 0xff) you are trying to mask the lower 8 bits of an integer (or a bigger type) but still the type passed stays the same and the behaviour of this function might be unpredictable. I've always found useful when applying masks to write them for the whole length of the variable im trying to mask, that is ie (AN4_sum & 0x00ff) so that it's always clear whats going on, specially if you are reviewing code long after it was written. Hope this helps a bit...

Answer 2

It sounds like you have inadequate adequate debugging support, making your job unnecessarily difficult.

Also, you mention several times "The problem", but you never tell us specifically what symptoms you observe, making it difficult for us to help you. Did an LED stay dark when you expected it to blink (or vice versa)? Does a display of 7-segment digits not show the number you expected?

If I were you,

1. I would read the datasheet for the proccessor I'm using, in particular the section "Using the Data EEPROM" page 116-117. It seems to say to set the WR bit after setting the EEPROM address and data registers, so the line "WR = 1;" at the beginning of a section of code seems out of place.

2. I would read the FAQ: "How do I write and read data from EEPROM using the XC8 compiler?"

3. I would read the User's Guide for the compiler -- "MPLAB XC8 C Compiler User’s Guide", in particular the section "EEPROM access functions" on p. 108.

4. When I have 2 chunks of code that do almost the same thing, I try to refactor them so I have 1 function that does all the common stuff, and I call that function from 2 places. In this case, you have (at least) 4 locations that write a 16-bit value to EEPROM, so I might refactor all 4 of them to call something like:

   void EEPROM_WRITE_16( char address, int value ){
       char MSByte = (value >> 0x08) & 0xff;
       char LSByte = value & 0xff;
       if(eeprom_read(address) != MSByte){
           eeprom_write(address, MSByte); // load MSbyte into given EEPROM address
       };
       address++;
       if(eeprom_read(address) != LSByte){
           eeprom_write(address, LSByte); // load LSbyte into following EEPROM address
       };
   }
   

5. Whenever I do stuff with EEPROM, I add a few "debug" routines in the firmware to read out the contents of the EEPROM and send them somewhere a human can read them -- perhaps to a UART debug port or, if one is available, a LCD or LED display; perhaps unconditionally, briefly on power-on.

6. Sometimes when debugging tough problems I add extra "debug" commands to write new values to EEPROM. If those functions call the same routine(s) that I use elsewhere to write to EEPROM, then I can test those routines more-or-less in isolation.

7. Sometimes I backup and set aside the main program, and create a new test program that starts out as an exact copy of the main program. As long as the test program exhibits the unexpected problem symptoms, I keep removing functions that I think are unrelated to the problem, until I end up with a SSCCE -- Short, Self Contained, Compilable, Example. Then I post that short test program when asking for help. Such a program generally gets a response much faster than either (a) posting the original, extremely long program, or (b) posting only fragments of the complete program that I think are related to the problem.

The last 3 steps (extra debug scaffolding and SSCCE) make it easier to quickly test various proposed solutions and see if they will really work for my code. (Suggestions like "loop until previous write is complete before starting a new read"(a) and page 108 of the "MPLAB XC8 C Compiler User’s Guide"; 'make sure the linker is set to "Link the Peripheral Library"'(b); '[replace] the C calls with the exact inline assembly instructions described in ... the datasheet'(c); etc.).

Good luck!