 |
 |
| View previous topic :: View next topic |
| Author |
Message |
dluu13
Joined: 28 Sep 2018
Posts: 395
Location: Toronto, ON
|
| pwm_set_duty_percent() turns off PWM |
 Posted: Tue Apr 28, 2020 9:17 am |
 |
|
Hi,
I am trying to get my head around this problem. I have a PWM channel set up. When I use #set pwm() the output defaults to 50% duty cycle. As soon as I call pwm_set_duty_percent() it turns off the pwm(). This has previously not done this before and it usually just continues outputting the same PWM. I have an earlier version of this program and it behaves the way I expect.
I'm using PIC24FJ128GA308, with PCD 5.085
I looked at the lst files and there is some difference:
New (turns off the pwm):
| Code: | .................... #pin_select OC1=AC_OUT
.................... #USE PWM(OC1, TIMER=2, FREQUENCY=10000, STREAM=CONC_PWM)
*
01118: MOV #0,W1
0111A: MOV 10C,W0
0111C: INC W0,W0
0111E: BTSC.B 42.1
01120: INC W1,W1
01122: PUSH 42
01124: BCLR.B 81.7
01126: SETM.B 42
01128: BSET.B 81.7
0112A: MOV 9BC,W2
0112C: MOV #0,W3
0112E: CALL 9E2
01132: BCLR.B 81.7
01134: POP 42
01136: BSET.B 81.7
01138: MOV #3E8,W2
0113A: MOV #0,W3
0113C: CALL 10C4
01140: MOV W0,196
01142: RETURN
...
... // lots of stuff here
...
// calling pwm_set_duty_percent() here
.................... void setup_conc_measure(void)
.................... {
.................... pwm_set_duty_percent(CONC_PWM, 500);
*
01144: MOV #1F4,W4
01146: MOV W4,9BC
01148: CALL 1118
0114C: RETURN
.................... }
|
old (does not turn off the pwm)
| Code: | .................... #pin_select OC1=AC_OUT
.................... #USE PWM(OC1, TIMER=2, FREQUENCY=10000, STREAM=CONC_PWM)
*
010EE: MOV #0,W1
010F0: MOV 10C,W0
010F2: INC W0,W0
010F4: BTSC.B 42.1
010F6: INC W1,W1
010F8: MOV 9B6,W2
010FA: MOV #0,W3
010FC: CALL 106A
01100: MOV #3E8,W2
01102: MOV #0,W3
01104: CALL 109A
01108: MOV W0,196
0110A: RETURN
...
... // lots of stuff here
...
.................... void setup_conc_measure(void)
.................... {
.................... pwm_set_duty_percent(CONC_PWM, 500);
*
0110C: MOV #1F4,W4
0110E: MOV W4,9B6
01110: CALL 10EE
01114: RETURN
.................... }
|
Since I never need to vary the PWM duty cycle for this certain application, it doesn't affect me if I just comment out the pwm_set_duty_percent() line, but I would like to know why there is this difference in behaviour.
If there is something quick and obvious then I would like to know what the problem is, but otherwise, I don't have time to troubleshoot and it's not holding me back for the moment. |
|
 |
jeremiah
Joined: 20 Jul 2010
Posts: 1315
|
|
| Posted: Tue Apr 28, 2020 12:12 pm |
|
|
It almost looks like you are trying to call the pwm functions in an interrupt in your other code (this is usually a code smell/no no)? It looks like it is doing operations on your INTCON registers. It might be a bug or it might be a logic error on your part.
It almost looks like they are using the nested interrupts bit to emulate disabling interrupts (if they somewhere had set the CPU to a different priority to all the other interrupts, this would work).
Without more context or a fully compilable small reproducable example, it might be hard to tell.
Other possibility: Did you change compiler versions between working and not working?
EDIT: I don't have your compiler version, but on mine, if I call pwm_set_duty_percent() inside the interrupt it definitely uses similar assembly (it's organized in a different order, but the same type of stuff). So this might be a logical issue in an interrupt for you. Maybe it is overriding your setting somehow. Can you post your interrupt handlers? |
|
|
dluu13
Joined: 28 Sep 2018
Posts: 395
Location: Toronto, ON
|
|
| Posted: Tue Apr 28, 2020 12:31 pm |
|
|
That is weird. This is definitely not called within an interrupt. Both pieces of code are compiled in the same compiler...
Furthermore, I call this code during initialization, even before I have enabled any interrupts.
Unfortunately I have a lot of non-code stuff to work on as well. I will try and see if I can find some time to troubleshoot this later on. |
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1315
|
|
| Posted: Tue Apr 28, 2020 3:38 pm |
|
|
| dluu13 wrote: | That is weird. This is definitely not called within an interrupt. Both pieces of code are compiled in the same compiler...
Furthermore, I call this code during initialization, even before I have enabled any interrupts.
Unfortunately I have a lot of non-code stuff to work on as well. I will try and see if I can find some time to troubleshoot this later on. |
There are a lot of gotchas for interrupt handlers. Most people don't even realize that certain types of code can cause this type of thing. On some PICs, something as benign as indexing a large array or doing multiplication can cause it if the compiler decides to create a scratch function to handle that. It could be that some other code in your interrupt handler is being shared along with the one in pwm_set_duty_percent(). Can you post up all of your interrupt handlers? Maybe someone will see something that could look benign to the inexperienced eye.
You could also email CCS support and see if they know what is going on. Maybe they would know what could cause it. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19215
|
|
| Posted: Wed Apr 29, 2020 2:00 am |
|
|
Big gotcha with pwm_set_duty_percent is the amount of maths this
actually involves. Think about it, you have a PWM 'period' as an int16
calculated by the #USE PWM, and you then ask for a percentage of this.
The code has to multiply and divide to work out the duty number needed.
result two maths operations which will lead to interrupts being disabled
if used in the interrupt handler....
The trapping for maths inside the interrupt handlers has been changed
several times in the last few compiler versions. Wonder if one of these
changes is jumping in here.... |
|
|
dluu13
Joined: 28 Sep 2018
Posts: 395
Location: Toronto, ON
|
|
| Posted: Wed Apr 29, 2020 6:49 am |
|
|
It really is a big gotcha...
I had one line with a "divide by 2" operation in one of my ISRs. That was the line that killed it. I figured that it wouldn't matter THAT much and it would only be a matter of speed but I guess it actually changes behaviour of some other code.
Thanks for the help. |
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1315
|
|
| Posted: Wed Apr 29, 2020 7:04 am |
|
|
| dluu13 wrote: | It really is a big gotcha...
I had one line with a "divide by 2" operation in one of my ISRs. That was the line that killed it. I figured that it wouldn't matter THAT much and it would only be a matter of speed but I guess it actually changes behaviour of some other code.
Thanks for the help. |
That's why you'll often see us harp on people to simplify their ISRs and try not to do much. So much can go wrong with them and some problems are incredibly difficult to diagnose (and may even seem completely random at times).
The hard problem is we are often tempted to just "stuff it in an interrupt" when we need something time sensitive to do. It's quick and it appears to work. And it might...or it might not. Sometimes the better (albeit harder/longer) method is to design your main software to be more responsive and do as little as possible in an ISR. There's a balance to strike with what goes in an interrupt handler. |
|
|
temtronic
Joined: 01 Jul 2010
Posts: 9097
Location: Greensville,Ontario
|
|
| Posted: Wed Apr 29, 2020 9:52 am |
|
|
hmm re: /2
probably not a problem with a byte ( simple 1 instruction for PIC)...
but.....
/2 on a 16 or 32 bit variable and man I bet the code gets 'complicated'.... |
|
|
dluu13
Joined: 28 Sep 2018
Posts: 395
Location: Toronto, ON
|
|
| Posted: Wed Apr 29, 2020 9:53 am |
|
|
| temtronic wrote: | hmm re: /2
probably not a problem with a byte ( simple 1 instruction for PIC)...
but.....
/2 on a 16 or 32 bit variable and man I bet the code gets 'complicated'.... |
Yes, I was doing it on a 32-bit variable.
How'd you guess? :P |
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1315
|
|
| Posted: Wed Apr 29, 2020 10:45 am |
|
|
| dluu13 wrote: | | temtronic wrote: | hmm re: /2
probably not a problem with a byte ( simple 1 instruction for PIC)...
but.....
/2 on a 16 or 32 bit variable and man I bet the code gets 'complicated'.... |
Yes, I was doing it on a 32-bit variable.
How'd you guess? :P |
It's a fairly common mistake.
32 bit variables are not atomic (16bit are not atomic on PIC18s and other 8bit micros). So referencing them both inside an interrupt handler and outside without synchronization primitives is technically "erroneous" or "undefined". They take multiple instructions to manage (instead of just one) and so if you modify them in the main and an interrupt occurs in the middle of the modification, the end result can be corrupted by the interrupt.
The compiler will not protect you from this. It will try to protect you from using "functions" this way, but not variables, so be aware. If you are accessing or modifying that same 32bit variable in non ISR code, then you are open to memory corruption unless you disable interrupts around the access or modification (there are other options, but that is the quickest work around to start with). |
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1315
|
|
| Posted: Wed Apr 29, 2020 10:57 am |
|
|
Consider this example:
| Code: |
static volatile unsigned int32 g_variable = 0;
#int_timer1
void timer1_isr(){
g_variable++;
}
void main(){
unsigned int32 temp;
while(TRUE){
temp = g_variable;
printf("%Lu\r\n",temp);
}
}
|
On a pic24, the assembly for the line temp = g_variable might look like:
| Code: |
MOV 0x800,W0 ; Point A
MOV 0x802,W1 ; Point B
MOV W0,0x804
MOV W1,0x806 ; Point C
|
Where g_variable is stored at 0x800-0x803 and temp is stored at 0x804-0x807
Now say that the value of g_variable is 0x0000FFFF and "Point A" executes, then the interrupt occurs. The interrupt will increment the variable to 0x00010000.
However, when "Point A" executed, it copied the FFFF into W0 already. Once the interrupt finishes (which increments the 0x0000FFFF to 0x00010000) it will execute "Point B" and copy the 0001 into W1 giving a combined result of 0x0001FFFF for the variable "temp" instead of either 0x0000FFFF or 0x00010000. The result stored in "temp" is now corrupted and has an erroneous and seemingly random value.
| Code: |
Time g_variable W1 W0 temp
-----------------------------------------------------------
Point A | 0x0000FFFF | 0x???? | 0xFFFF | 0x????????
Interrupt | 0x00010000 | 0x???? | 0xFFFF | 0x????????
Point B | 0x00010000 | 0x0001 | 0xFFFF | 0x????????
Point C | 0x00010000 | 0x0001 | 0xFFFF | 0x0001FFFF
|
And that's just reading the variable. Imagine if that is just one step in a complex math operation done by the compiler or instead of reading the variable you are also modifying it as well. It can really snowball.
However, you would only see this at the times when the interrupt overlaps with that copy operation. That may only happen 1 in 1000 times or even less likely. So you may never see it until it is too late (in a fielded product).
The simple solution is to disable interrupts around it:
| Code: |
static volatile unsigned int32 g_variable = 0;
#int_timer1
void timer1_isr(){
g_variable++;
}
void main(){
unsigned int32 temp;
while(TRUE){
disable_interrupts(GLOBAL);
temp = g_variable;
enable_interrupts(GLOBAL);
printf("%Lu\r\n",temp);
}
}
|
That can be considered heavy handed sometimes though. If you are only reading the variable in non ISR code, then you can also do something like:
| Code: |
static volatile unsigned int32 g_variable = 0;
#int_timer1
void timer1_isr(){
g_variable++;
}
void main(){
unsigned int32 temp;
while(TRUE){
do{
temp = g_variable;
}while(temp != g_variable);
printf("%Lu\r\n",temp);
}
}
|
This works only for reading. Basically it detects the corruption and executes the code another time to get a valid reading. This allows you to read (and only read) the variable without disabling interrupts. |
|
|
dluu13
Joined: 28 Sep 2018
Posts: 395
Location: Toronto, ON
|
|
| Posted: Wed Apr 29, 2020 11:33 am |
|
|
On PIC24 changing a 16bit variable is an atomic operation right? Or am I still missing something?
See, I had vaguely heard about this stuff in a class once (years ago)... When I first started with PICs I had been pretty careful in trying to disable interrupts around certain parts of my code but I guess I forgot about that over time.
Thanks for the detailed example. |
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1315
|
|
| Posted: Wed Apr 29, 2020 11:37 am |
|
|
| dluu13 wrote: | On PIC24 changing a 16bit variable is an atomic operation right? Or am I still missing something?
See, I had vaguely heard about this stuff in a class once (years ago)... When I first started with PICs I had been pretty careful in trying to disable interrupts around certain parts of my code but I guess I forgot about that over time.
Thanks for the detailed example. |
16bit variables are indeed atomic on PIC24 and dsPIC (not for PIC10-PIC18 though). 32bit variables are not atomic on PIC24s however. You can read and write to atomic variables without worry of data corruption.
Note however, that operations on 16bit variables may not necessarily be atomic:
| Code: |
static volatile unsigned int16 g_variable = 0;
|
| Code: |
g_variable++; // NOT Atomic!! g_variable = g_variable + 1;
|
The value could be incremented, then an interrupt occurs, and then the assignment happens. Now the result may not be what you want if the interrupt also modifies g_variable. It might be ok or it might not, depends on what you logic needs.
This example is called a "race condition" because it isn't technically data corruption and is instead a logic error. My previous post highlighted what is called a "data race" which results in erroneous/corrupted/undefined data. |
|
|
|
|
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
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
