|
|
View previous topic :: View next topic |
Author |
Message |
Woody
Joined: 11 Sep 2003 Posts: 83 Location: Warmenhuizen - NL
|
:grin: I learned something today! |
Posted: Sat Dec 06, 2003 9:33 am |
|
|
Hi,
This is probably something that a lot of people in this forum will say 'Oh well, what's new?' about. But it kept me off the street for half a day, so I'll tell you anyway. Maybe it will help someone else.
I have an application using a 12F675 that counts pulses from an index wheel, powered by a motor switched by the PIC. This wheel turns X times (programmable). The wheelsensor generates an interrupt-on-change. In the interrupt routine a counter (pulse_counter) gets decremented. Pulse_counter is a long, for I need it to be able to contain around 500. In the main loop I set pulse_counter once, start the motor and wait for pulse_counter to reach 0 before I switch the motor off again. I do that by testing: if (pulse_counter == 0x0000)
This works flawless in the initial stages of the project. Then I start to use larger numbers, more in the range of those used in the actual app. And suddenly, every now and then the motor stops way before the correct number of turns has been done.
I try to find some logic in the occurence of the bug, but fail. I then connect a serial rs232 display to my one and only leftover PIC pin to be able to look at the actual value of pulse_counter. I see that when my bug occurs, pulse_counter reads 0x00ff. That starts me thinking, how can the test in the main loop decide that 0x00ff is 0?
Here's how:
When you test a long, the compiler generates code that tests the two bytes of this long separately (obviously). First it loads the LSB, and tests it for 0. If 0, then it tests the MSB. If that is 0 also, the long is 0.
Enter the interrupt. Assume you get an interrupt during this test, between testing the LSB and the MSB. Also, assume the value of your long to be 0x0100. The LSB test sees 0x00, and jumps to test the MSB. Then the interrupt occurs. In the interrupt routine the long gets decremented to 0x00ff. Back to the code where the interrupt occured. The MSB is loaded and tested. This reads also 0 now. So the long reads as 0 now. And my motor gets shut off.....
As I said, I expect that lots of people on this forum find this so logical as not to mention it, but I did not realise this myself and it made me very happy to finally find the source of this intermittent error. I still wonder if CCS could solve this by generating the test the other way around (first the MSB and then the LSB) but that would probably lead to other problems.
Woody. |
|
|
Mark
Joined: 07 Sep 2003 Posts: 2838 Location: Atlanta, GA
|
|
Posted: Sat Dec 06, 2003 9:46 am |
|
|
You might not believe this but I had the exact same problem and didn't discover it until about 12:00am this morning!
Code: |
000504 0100 MOVLB 0x0 while(TMR0Count) { }; C:\PIC18\Ringtone\ringtone.c
000506 516d MOVF 0x6d,0x0,0x1
<-------- Int occurs here changing TMR0Count
000508 116e IORWF 0x6e,0x0,0x1
00050a e001 BZ 0x50e
00050c d7fb BRA 0x504
|
Same thing as you. When TMR0Count = 0x100 it will fail! Now I am using the C18 compiler for this project but the same problem still happens. It would be nice if CCS and Microchip were to add a 'nop between the 2 reads. This could be an option if the variable were declared as 'volatile'. Or I guess they could switch the order of the read. Read the high byte first and then the low byte.
Last edited by Mark on Sat Dec 06, 2003 10:21 am; edited 1 time in total |
|
|
Testicky
Joined: 22 Nov 2003 Posts: 12 Location: South Africa / Taiwan
|
copy your LSB and MSB before you test them |
Posted: Sat Dec 06, 2003 9:50 am |
|
|
Why don't you make a copy of your registers and test the copied registers for 0, this way, their value won't change. _________________ Testicky |
|
|
Mark
Joined: 07 Sep 2003 Posts: 2838 Location: Atlanta, GA
|
|
Posted: Sat Dec 06, 2003 10:20 am |
|
|
Er...Because the program would lock up. The code is waiting for the value to hit 0. It must change! |
|
|
Woody
Joined: 11 Sep 2003 Posts: 83 Location: Warmenhuizen - NL
|
|
Posted: Sat Dec 06, 2003 10:26 am |
|
|
Hi,
I solved this by testing in the interrupt routine as well. I then signal the result of this test via one bit in a flag byte that I later test in the main loop. I use that often in my code. Works great.
Woody |
|
|
KerryW Guest
|
This happens a lot. |
Posted: Sat Dec 06, 2003 10:29 am |
|
|
I fall for it regularly. When I figure it out, I always say "Oh, yeah, that one again."
Disable the interrupts before making the compare, then re-enable them. |
|
|
Dirk Guest
|
Re: :grin: I learned something today! |
Posted: Sat Dec 06, 2003 11:47 am |
|
|
Using interrupts can be tricky!
When you change multibyte variables both in interrupt and in main code, you need to disable interrupts.
A better solution is to avoid chaging multbyte varables in two interrupts levels altogether, and to pass counts through a single byte variable.
If the multibyte variable gets changed only during interrupt time, you could test it there and set a flag for the main code when it hits 0.
Dirk |
|
|
Neutone
Joined: 08 Sep 2003 Posts: 839 Location: Houston
|
Re: copy your LSB and MSB before you test them |
Posted: Sat Dec 06, 2003 1:19 pm |
|
|
Testicky wrote: | Why don't you make a copy of your registers and test the copied registers for 0, this way, their value won't change. |
That would just reduce the chance of an error not eliminate the chance for an error. Say you copy the high byte as 0, the interupt occurs and sets the low byte to 0 and the high byte to non-zero, then you copy the low byte as 0 as well and fail the test for zero. Doing a zero check is fast and not unreasonable for an interupt operation that sets a flag. |
|
|
Mark
Joined: 07 Sep 2003 Posts: 2838 Location: Atlanta, GA
|
|
Posted: Sat Dec 06, 2003 1:44 pm |
|
|
I have no problem figuring a way AROUND the problem! I am saying that the compiler manufacturers can change the way they do the test so we don't have to do a "work around". |
|
|
Ttelmah Guest
|
|
Posted: Sat Dec 06, 2003 3:35 pm |
|
|
Mark wrote: | I have no problem figuring a way AROUND the problem! I am saying that the compiler manufacturers can change the way they do the test so we don't have to do a "work around". |
Except, that this would involve a time penalty, which is unecessary in many cases. For instance, your external test, may be designed to be synchronous to the interrupt event, and allways take place after the interrupt has triggered, but before it happens again. To add protection around a test, would involve extra instructions, that in 99% of cases will be wasted.
The answer actually 'comes down', to learning about the hardware abilitites and limitations of the chip, before programming, when working on an embedded application. This applies to many of the problems seen.
Best Wishes |
|
|
Mark
Joined: 07 Sep 2003 Posts: 2838 Location: Atlanta, GA
|
|
Posted: Sat Dec 06, 2003 4:32 pm |
|
|
I know the chip very well and understand the problem thoroughly! There would be no time penality if the compiler were to switch the order of the reads. Now the code I posted was from the C18 compiler. CCS might not do it the same way. I am not sure. The problem is that the low byte is checked first. If the value were say 0x100 then the low byte would be 0. Now the int occurs and the new value is 0x00FF. Now the high byte is checked and found to be 0 and Houston we have a problem. Now lets reverse the order. The value is 0x100. The high byte is non zero so we continue to wait. Now the value is 0x00FF. The high byte is 0 so check the low byte. It won't be 0 until the value is 0! No penality and works for a zero/non-zero test which is quite common. As for the suggestion of adding the nops, I don't remember why that would do any good Maybe I had a reason or maybe I just stayed up too late! But what I was trying to say was that this special code would only be added if the variable were declared as volatile meaning that it can change from some outside source. |
|
|
Ttelmah Guest
|
|
Posted: Sun Dec 07, 2003 4:07 am |
|
|
Mark wrote: | I know the chip very well and understand the problem thoroughly! There would be no time penality if the compiler were to switch the order of the reads. Now the code I posted was from the C18 compiler. CCS might not do it the same way. I am not sure. The problem is that the low byte is checked first. If the value were say 0x100 then the low byte would be 0. Now the int occurs and the new value is 0x00FF. Now the high byte is checked and found to be 0 and Houston we have a problem. Now lets reverse the order. The value is 0x100. The high byte is non zero so we continue to wait. Now the value is 0x00FF. The high byte is 0 so check the low byte. It won't be 0 until the value is 0! No penality and works for a zero/non-zero test which is quite common. As for the suggestion of adding the nops, I don't remember why that would do any good Maybe I had a reason or maybe I just stayed up too late! But what I was trying to say was that this special code would only be added if the variable were declared as volatile meaning that it can change from some outside source. |
This would not solve the problem.
Reversing the byte order, reverses the count direction in which the problem occurs, but does not get rid of the problem. It'll now fail for 'up' counters, rather than down. However if you want to compare this way, it is easily done, by generating a 'wrapper' function. So:
union access {
int16 word;
int8 b[2];
};
int compare_high_first(union access val1,union access val2) {
if (val2.b[1]!=val1.b[1]) return false;
if (val2.b[0]!=val1.b[0]) return false;
return true;
}
This produces very nearly the same code as the normal compare, but with the byte order reversed.
To genuinely cure the problem in both directions, involves disabling the interrupt during the two reads.
Best Wishes |
|
|
Mark
Joined: 07 Sep 2003 Posts: 2838 Location: Atlanta, GA
|
|
Posted: Mon Dec 08, 2003 7:42 am |
|
|
Note that I said
Quote: |
No penality and works for a zero/non-zero test which is quite common
|
Reversing the order fixes the problem here
MOVLB 0x0 while (Note);
MOVF 0x6f,0x0,0x1
IORWF 0x70,0x0,0x1
BZ 0xba6
BRA 0xb9c
Reversing the order fixes the problem here
MOVLB 0x0 while (!Note);
MOVF 0x6f,0x0,0x1
IORWF 0x70,0x0,0x1
BNZ 0xbb0
BRA 0xba6
Don't really care about a case where the value is changing and I am looking for a specific value.
MOVLB 0x0 while (Note == 0x100);
MOVF 0x6f,0x0,0x1
BNZ 0xbba
MOVLW 0x1
XORWF 0x70,0x0,0x1
BNZ 0xbbe
BRA 0xbb0
Now with a little different technique they could fix the other cases also. Note that it might add 1 or 2 instructions but it is worth it in my opinion simply from the fact that I have to add code to make it work if they don't do it. This could be also controlled by the volatile declaration also so that non isr changing values would not have any "extra" code.
This would be a counting up case
All that really needs to be checked is the high byte any time the low byte is 00.
MOVLB 0x0 while (Note < 0x100);
MOVLW 0x0
SUBWF 0x6f,0x0,0x1
MOVLW 0x1
SUBWFB 0x70,0x0,0x1
BC 0xbcc
BRA 0xbbe
This would be a counting down case
Check the high byte to make sure that it is >=1 if it is, then check the low byte to make sure that it is > 0
Reversing the order fixes the problem here
MOVLB 0x0 while (Note > 0x100);
MOVLW 0x0
BSF 0xd8,0x0,0x0
SUBFWB 0x6f,0x0,0x1
MOVLW 0x1
SUBFWB 0x70,0x0,0x1
BC 0xbdc
BRA 0xbcc |
|
|
Ttelmah Guest
|
|
Posted: Mon Dec 08, 2003 8:27 am |
|
|
Mark wrote: | Note that I said
Quote: |
No penality and works for a zero/non-zero test which is quite common
|
Reversing the order fixes the problem here
MOVLB 0x0 while (Note);
MOVF 0x6f,0x0,0x1
IORWF 0x70,0x0,0x1
BZ 0xba6
BRA 0xb9c
|
Only if you are counting in ones. The problem _still remains_ if the 6F byte can change between the two lines.
Quote: |
Reversing the order fixes the problem here
MOVLB 0x0 while (!Note);
MOVF 0x6f,0x0,0x1
IORWF 0x70,0x0,0x1
BNZ 0xbb0
BRA 0xba6
|
Same comment.
Quote: |
Don't really care about a case where the value is changing and I am looking for a specific value.
MOVLB 0x0 while (Note == 0x100);
MOVF 0x6f,0x0,0x1
BNZ 0xbba
MOVLW 0x1
XORWF 0x70,0x0,0x1
BNZ 0xbbe
BRA 0xbb0
Now with a little different technique they could fix the other cases also. Note that it might add 1 or 2 instructions but it is worth it in my opinion simply from the fact that I have to add code to make it work if they don't do it. This could be also controlled by the volatile declaration also so that non isr changing values would not have any "extra" code.
This would be a counting up case
All that really needs to be checked is the high byte any time the low byte is 00.
MOVLB 0x0 while (Note < 0x100);
MOVLW 0x0
SUBWF 0x6f,0x0,0x1
MOVLW 0x1
SUBWFB 0x70,0x0,0x1
BC 0xbcc
BRA 0xbbe
|
It is not all that is needed. You are assuming that the count is again in single units. This is not a generic solution, and you allready have two different routines being needed for the different cases. You can test this way yourself, but none is a generic solution to the problem. Disabling the interrupts is the only really safe solution, and you can do this yourself...
Quote: |
This would be a counting down case
Check the high byte to make sure that it is >=1 if it is, then check the low byte to make sure that it is > 0
Reversing the order fixes the problem here
MOVLB 0x0 while (Note > 0x100);
MOVLW 0x0
BSF 0xd8,0x0,0x0
SUBFWB 0x6f,0x0,0x1
MOVLW 0x1
SUBFWB 0x70,0x0,0x1
BC
0xbdc
BRA 0xbcc
|
All your examples, only cover individual specific cases. Since there is nothing 'magic' about counting downwards (indeed the PIC itself counts upwards), there is no reason why you cannot just count in the opposite direction, which fixes the problem for the 'unity count' solution.
If you add 'automatic' interrupt disabling, you are adding extra code that may be uneccessary in many cases. Using 'shadow' storage for interrupt counters, involves a RAM overhead, and code overhead as well.
Basically, you are asking CCS to modify the compiler to suit a particular useage you have, which may well cause problems in other situations (I would not want interrupts disabled for each test involving a number accessed both inside and outside an ISR), when this can already be done by coding carefully yourself.
Best Wishes |
|
|
Mark
Joined: 07 Sep 2003 Posts: 2838 Location: Atlanta, GA
|
|
Posted: Mon Dec 08, 2003 8:41 am |
|
|
I only suggested the test for zero/ nonzero changed. I never said a damn thing about CCS enabling/disabling interrupts! All the code I posted was from the C18 compiler not CCS.
Quote: |
Only if you are counting in ones. The problem _still remains_ if the 6F byte can change between the two lines.
|
I don't care what units I am counting in. If I check the high byte first and it is a non zero value, then it doesn't matter what the low value is!
Bad way:
Code: |
MOVLB 0x0 while (Note);
MOVF 0x6f,0x0,0x1
IORWF 0x70,0x0,0x1
BZ 0xba6
BRA 0xb9c
|
Better way
Code: |
MOVLB 0x0 while (Note);
MOVF 0x70,0x0,0x1
IORWF 0x6f,0x0,0x1
BZ 0xba6
BRA 0xb9c
|
Quote: |
Basically, you are asking CCS to modify the compiler to suit a particular useage you have, which may well cause problems in other situations (I would not want interrupts disabled for each test involving a number accessed both inside and outside an ISR), when this can already be done by coding carefully yourself.
|
Again, I don't care what they do. I just said it would be better! If I wanted them to change it, I would have emailed them! And again, if they wanted to change it, they could have their normal case and then a special case when the variable is volatile. |
|
|
|
|
You cannot post new topics in this forum You cannot reply to topics in this forum You cannot edit your posts in this forum You cannot delete your posts in this forum You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|