Array Values Resetting When Called From ISR

[Ground_Loop]

I have a routine that needs to store values when a power outage is detected.  This is being done in a PIC18F27Q10 through MPLAB X.

Function storeData() is declared in main.h, defined in main.c and included in HLVD.c via main.h

Code: [Select]

void
storeData(void)
{
DATAEE_WriteByte(EEaddress_1, a[0]);
DATAEE_WriteByte(EEaddress_2, a[1]);
DATAEE_WriteByte(EEaddress_3, a[2]);
DATAEE_WriteByte(EEaddress_4, a[3]);
IO_RC3_SetHigh();
}
Setting RC3 is a brief visual indicator until power fully fades that the routine executed.

The array a[] is a global, declared and defined in main.c ahead of main()

Code: [Select]

unsigned char a[5];

void
main(void)
{
The values are also arbitrarily stored every 300 loop counts within main.c.  This works:

Code: [Select]

if (loopCount >= 300) {
storeData();
loopCount = 0;
}
When called from the HLVD ISR, the array a[] resets

Code: [Select]

void
HLVD_ISR(void)

{
LED_1_SetLow(); // shutdown unnecessary load
storeData();
PIR2bits.HLVDIF = 0;
}
I have tried static, volatile, passing the array as a parameter, etc and in all cases the array resets when called from the ISR, but works fine when called from within main.c.  It also works if I substitute constant values for array values, so it's not a power loss problem.  I'm sure this is the result of some fundamental misunderstanding. Please help

[SiliconWizard]

I've never used HLVD on PIC18 MCUs, but looked at the manual.

The voltage threshold can be configured. How did you configure it? What could happen here is if the threshold is set too low, the MCU could be failing to run properly when the condition occurs, memory content could begin to be lost, etc. So, what threshold did you set?

It also works if I substitute constant values for array values, so it's not a power loss problem.

Not sure what makes you claim that.

Another point to consider: how and where do you intialize the content of the 'a' array?

[Ground_Loop]

I've never used HLVD on PIC18 MCUs, but looked at the manual.

The voltage threshold can be configured. How did you configure it? What could happen here is if the threshold is set too low, the MCU could be failing to run properly when the condition occurs, memory content could begin to be lost, etc. So, what threshold did you set?

It also works if I substitute constant values for array values, so it's not a power loss problem.

Not sure what makes you claim that.

Another point to consider: how and where do you intialize the content of the 'a' array?

I put constant values in the function instead of using the array and then killed power.  Upon restart the constant values had been written to memory. That's how I make that claim.

I have also set a flag bit in the ISR that is them used to trigger the function call in main(). This works too usually. The problem with that method is the call timing depends on where main is in it's execution when the ISR returns. Sometimes not enough power remains to execute the memory write.

HLVD threshold is set at 4VDC.

I can verify the routine is executing because the led flashes momentarily at the end of the memory write. Further, upon restart the memory location that once held values (recall main() also writes to the same locations every 300 loops) have be overwritten with zero.  So there is no doubt that the array values are being reset to zero when called from the ISR.

[NorthGuy]

The values are also arbitrarily stored every 300 loop counts within main.c.

This may exhaust EEPROM. You can only write 100k times guaranteed, after that EEPROM may stop working.

[SiliconWizard]

I put constant values in the function instead of using the array and then killed power.  Upon restart the constant values had been written to memory. That's how I make that claim.

The reason I asked is because this is no proof by itself the MCU is *fully* operational. It just means it's still able to execute this small piece of code properly enough. We'd need to see the compiled assembly code for the ISR, but using constants instead of reading a[] is likely to yield smaller code, and with no RAM access, so that *could* be one reason it appears to work in this case and not when you read the values from a[]. It could work just because the code is smaller and thus takes less time to execute, it could also be that the RAM content at this point is already borked.

So, just to make clear that just because something appears to work in some cases and not others, doesn't mean that there's nothing wrong when it does appear to work. This is a common trap actually. Generally speaking, as long as you haven't fully understood a "bug", even in cases where it appears to "work", there may still be something fishy lurking around. That's why "fixing" a bug without understanding it, which is unfortunately much too common, is rarely a good idea. It's a good recipe for the bug coming back at some point to haunt you.

With that said and set aside, with a 4V threshold, after reading the datasheet, it's rather unlikely to cause issues as the min operating voltage of this MCU is 1.8V, so you should have a large margin. That said, we don't know what frequency it runs at. I haven't read the DS thoroughly, but I doubt, for instance, that it can run at the max frequency at 1.8V, for instance. There's often a minimum voltage for a given operating frequency. So there's another information missing here.

Another missing info is how fast the supply voltage decreases when you get into this "low voltage" condition. Have you looked with a scope? Reason I ask is, for instance, if there's not enough bulk capacitance on the power rail, the voltage may drop so fast that from the moment it gets below 4V and the moment it gets below a safe operating voltage for the MCU, the ISR may not be done executing. That may not be what's happening here, but something to consider.

Another missing info (that your forgot to answer) is how you initialize the content of 'a[]'.
You showed its declaration ("unsigned char a[5];"), but not how its content is being populated. We can only guess it's being written to at run time, maybe somewhere in your main() function.
If there's not particular problem with any of the points I talked about above, this could be something else to investigate: if you declare a global arrray like this, it's going to be initiazed upon startup with all zero's (it's guaranteed by the C language, and is usually done by the startup code that executes before your main() function and calls it.) What that means is that, between the point where the MCU resets, and the point where you actually populate 'a[]' at run-time, it will contain only zero's. Now we may be on to something.

Have you enabled the BOR and/or LPBOR? If so... keep reading.

Then one possible scenario for what you see could be:
- Voltage drops below 4V => ISR gets called => a[] contains the 'right' values and they get properly written to EE, assuming voltage was still high enough to completion;
- Voltage keeps dropping, and the MCU resets due to a BOR or LPBOR condition;
- Voltage may bounce up a bit while the MCU resets, just because during reset the MCU will draw a lot less power;
- So the MCU may restart after the BOR, but at this point, if the voltage is still below 4V, the HLVD ISR will be called almost first thing BEFORE your code got a chance to initialize 'a[]'.

To check that, you may toggle another IO before and after the initialization of 'a[]'. Looking at this IO plus the IO that you toggle in the ISR with a scope will give you much more info than you currently have.

[Ian.M]

Also the array a[] should be volatile, not just global, otherwise there's no guarantee that the compiler won't optimize writes to it away, storing its contents in 'scratch' RAM or even discarding them, not realizing that the ISR (another thread of execution) accesses the same object.

[SiliconWizard]

Also the array a[] should be volatile, not just global, otherwise there's no guarantee that the compiler won't optimize writes to it away, storing its contents in 'scratch' RAM or even discarding them, not realizing that the ISR (another thread of execution) accesses the same object.

Ah, good point. If the array is only read within the ISR, and not qualified 'volatile', then yes, the compiler may assume it's never accessed. Not seeing any of the code except very small fragments as above doesn't help. But that could well be the reason, very common trap! In that case, the fix is to declare it like so:

Code: [Select]

volatile unsigned char a[5];
That said, even if it is that, I'd still suggest to consider what can happen in the scenario I talked about above. In particular, what happens if Vdd bounces back up, which is not unlikely to happen especially if running off batteries.

Something obvious to avoid this trap is to enable the HLVD interrupt *only* after the a[] array is fully intiliazed. Maybe this is already what the OP does. If not, something to fix.

But, even so, I'd be cautious with what NorthGuy mentioned; although it's not likely to be your problem here from your description, it is still a concern for long-term operation. You'll definitely need to restrict the number of writes drastically to avoid killing the EE memory over time; given that "Data Memory EEPROM" endurance is spec'ed at 100k cycles which is not all that much. Especially if you keep writing at the same location.

[agehall]

Reading your first post, I thought you had tried to make the array volatile. If that wasn’t the case, then that is most likely the issue! Rule #1 when having variable you want to access outside of the normal code flow, is to make them volatile.

[Ground_Loop]

I thought I already went the volatile route, but I'll try again. I understand the memory write limit and have considered moving the location around but I only expect to store these values about twice a week during normal operation and then any time I loose power. 100K writes get me many decades of use.  Thanks for all the replies. Definitely some things to consider.

[artag]

The memory is banked (see section 10.3.1 of PIC18F27-47Q10-Data-Sheet-40002043E.pdf).

Are you sure the correct bank is selected when you execute the ISR ?
The C environment may manage banks for you in non-ISR code but probably not in an ISR.

[Ground_Loop]

No joy on volatile.

[Ian.M]

Leave the volatile attribute in anyway.     *ALWAYS* use it for *ALL* variables shared between different threads of execution (e.g. main program and various ISRs) to avoid giving the compiler's optimizer any excuse to screw things up.

Its probably time to start poking through the .lst file to see if you can spot any differences between EEPROM access from the ISR and from the main program.

Also consider running under a hardware debugger and moving your HVLD ISR code to an ordinary INT pin ISR so you can trigger it manually and set a breakpoint in the ISR, without browning out the PIC and/or loosing debugger comms.  N.B. MPLAB does *NOT* live update the EEPROM window in the IDE when debugging (it does when simulating).

[Ground_Loop]

The memory is banked (see section 10.3.1 of PIC18F27-47Q10-Data-Sheet-40002043E.pdf).

Are you sure the correct bank is selected when you execute the ISR ?
The C environment may manage banks for you in non-ISR code but probably not in an ISR.

The address U,H and L bytes are calculated in the EEWrite function based on the desired location with the Upper byte being fixed at 0x31.  So, it looks ok from a banking standpoint.

Code: [Select]

void DATAEE_WriteByte(uint16_t bAdd, uint8_t bData)
{
    uint8_t GIEBitValue = INTCONbits.GIE;
   
    //Set NVMADR with the target word address: 0x310000 - 0x3103FF
    NVMADRU = 0x31;
    NVMADRH = (uint8_t)((bAdd & 0xFF00) >> 8);
    NVMADRL = (uint8_t)(bAdd & 0x00FF);

[Tagli]

What if HLVD interrupt triggers while the periodic storeData() is in progress?

AFAIK, normal functions in XC8 are not reentrant. However, when a function is shared by the main thread and an interrupt, actually two functions having the same binary are created, somewhat similar to function templates in C++. But still, storeData() isn't thread safe, as the EEPROM is a resource and should not be accessed by two threads at the same time.

[artag]

The address U,H and L bytes are calculated in the EEWrite function based on the desired location with the Upper byte being fixed at 0x31.  So, it looks ok from a banking standpoint.

Not the EE banks. the sram banks.

[Ground_Loop]

Threw in the towel on this one and went back to setting a flag in the ISR then running the function from main(). It works most of the time, but is not the right solution. I tried several other methods including individual variables instead of the array. Nothing I tried would result in usable data in the ISR.

[bson]

What does the ISR return to?  Business as usual?  If it's a brownout ISR it can probably loop until the supply is stable again (or never if it results in a BOR).

[SiliconWizard]

I don't know if you have a scope, and I kind of suspect you don't. Having one would tremendously help debugging this, as I suggested earlier. Without one, you're shooting a bit in the dark.

[ucanel]

I had ran into similar problem
long story short:
1) Do not write flash (mine was internal Eeprom) memory of a Pic mcu in ISR.
2) Do not write flash memory of a Pic running lower than it's working frequency required Vdd voltage.

[Ground_Loop]

I don't know if you have a scope, and I kind of suspect you don't. Having one would tremendously help debugging this, as I suggested earlier. Without one, you're shooting a bit in the dark.

I might have a scope or two
And a couple more squirreled away out of frame. I have analyzed the voltage decay time vs when the routine executes by firing IO at entry and exit. Entry voltage is 4V as expected. Exit voltage is about 2.8. I have not monitored the clock out to check frequency decay. As stated below I have a pile of capacitance to soften the power outage. Also note that I'm doing this for the fun of it. Not my day job.  Learning is always good.

[Ground_Loop]

What does the ISR return to?  Business as usual?  If it's a brownout ISR it can probably loop until the supply is stable again (or never if it results in a BOR).

the ISR returns to the last point of execution followed shortly by complete power loss. I have enough rail capacitance to hold up the MCU long enough to finish up the housekeeping tasks.

[viperidae]

Can you set a breakpoint in the isr and trigger it by dropping the voltage to say, 3.5v, so the MCU doesn't stop working?

I'd also check the assembly listing to make sure the isr code is using the correct memory address for the variable. Like someone else has mentioned, these 8 bit pics have banked ram, the isr code needs to set the correct memory bank as it could be anything when the code runs.

I would have assumed the compiler would manage the memory bank though

[DavidAlfa]

 I know you already tried, but add "volatile" to anything that is read or modified inside an interrupt.
Otherwise the compiler might optimize too much because it's not aware of such situation.

For example, I had trouble with something as simple as this.
The while would stay forever.
The compiler assumed that, as you set Test to 1 and nothing else could change it inside the function, it was a while(1).
After setting Test as volatile, all worked again.
A lot of strange things can happen not doing so.

Code: [Select]

char Test;

Void testFunction(void){
    Test = 1;
    something();
    while(Test);
}

Void something_Interrupt(void){
  Test=0;
}
Do your saving function depend on other interrupts?
You might have an interrupt priority issue.
Try setting a global flag, ex. PowerOff, and set only that inside the ISR.
And in the main loop, something like
if(PowerOff){ SaveData(); }

Keep good programming practices, ex avoiding big while delays and replacing them with if and machine states.

Also, your code is not interrupt proof. You might be writing the data from the 300loop. While writing, you trigger the interrupt and bang! You call again save settings.
Why save the data every 300 cycles. You will stress the eeprom and cause wear for nothing.
Just do It from the interrupt, when the power goes off.
If you make it right, you should never lose anything.

you should ensure that the power is holding enough time to save everything.
Put a low voltage schottky to the Pic supply and a decent capacitor.So the Pic power can't be sucked by other devices when the supply goes down.
Don't connect anything else to the Pic VDD.
Post more details to better assist you