PWM on arduino UNO pin3 breaking USB output

Hi,

I seems to be missing something about setting up a PWM on arduino UNO pin3.

I've been pouring over atmel doc for days trying to untangle all the various cryptic bit and register names and thought I had it sussed. Apparently there's still something I'm missing.

I have a loop like the Serial_Fading example reading a ch from serial, then I call stopPWM and startPWM on getting s or g respectively.

The loop works and I see the debug stop and start messages but not a clean pulse on pin3.

Could someone more familiar with the hieroglyphs ease my pain?

Thanks.

void stopPWM() {
  cli();
  // disable interrupts
  bitWrite(TIMSK2, OCIE2A, 0); // bit 1: o/p compare reg A used by WGM
  bitWrite(TIMSK2, TOIE2, 0);  // bit 0: overflow event used by WGM
 
 //Hi_off; Lin off;
 sei();
  Serial.println("stopPWM: off");
} // end stopPWM()


void startPWM() {
  cli();
  // enable interrupts for WGM
  bitWrite(TIMSK2, OCIE2A, 1); // bit 1: o/p compare reg A used by WGM
  bitWrite(TIMSK2, TOIE2, 1);  // bit 0: overflow event used by WGM
  sei();
   Serial.println("startPWM: on");
} // end startPWM();


ISR( TIMER2_OVF_vect ){
  // if pwd_change
  //   analogWrite(PWM_pin, new_pwm_value); // this is calculated in P&O funct. ###
  // lo off ; pulse(Hin);
  // implicit reti();
}

ISR( TIMER2_COMPA_vect ){
  // hi off ; pulse(Lin);
}


byte const PWM_pin=3;  // ~ = 3.5.6.9.10.11 ; 10.11 on SD ;  Pins 11 and 3: controlled by timer2
byte const pwm_min=4;
byte const pwm_max=250; // min, max ensure bootstrap still charges.


void setup_PWM(){
  cli();
  // http: //letsmakerobots.com/node/28278 
  pinMode(PWM_pin, OUTPUT); // input still does PWM interrupts but not physical o/p
//  TCCR2C; regC 'force comp' bits not needed for PWM modes; 
//  TCCR2B[2..0]: 3LSB of regB = prescalarbits;
//  TCCR2B[7,6] : input capture only, noise filter , edge detection. [5] n/c
//  TCCR2[A,B]: 8bit fast pwm: WGM2[3..0] = TCCR2B[4,3] + TCCR2A[1.0] = 0101; 

// also GTCCR to sync different counters
// OCR2[A,B] output comp registers
// OC2[A,B] output pins [ arduino pin3,pin11 ]

  TCCR2A = 0;
  TCCR2B = 0;
  TCCR2A |=  0b01;  // WGM2[1,0] 
  TCCR2A |=  0b10000000;  // COM2A[1,0]= TCCR2A[7,6]  = 10 Clear OC2A on Compare Match, set OC2A at BOTTOM
  
//  TCCR2B |=  0b01001;  // WGM2[3,2] + full 16MHz
  TCCR2B |=  0b01010;  // WGM2[3,2] + div 8 = 2MHz clk -> 8kHz PMW

//  enable output compare: OCIE2y bit in the interrupt mask register TIMSK2; ISR(TIMER2_COMPy_vect)
//  enable timer overflow: TOIE2 bit in the interrupt mask register TIMSK2; ISR(TIMER2_OVF_vect)  
//  enable input capture : ICIE2 bit in the interrupt mask register TIMSK2; ISR(TIMER2_CAPT_vect) 
  bitWrite(TIMSK2, OCIE2A, 1); // bit 1: o/p compare reg A used by WGM
  bitWrite(TIMSK2, TOIE2,  1);  // bit 0: overflow event used by WGM
  bitWrite(TIMSK2, OCIE2B, 0); // bit 2: p/p compare reg B :off

//  analogWrite(PWM_pin, pwm_min); // ensure finite width for bootstrap diode pulse.eg 8kHz, 2us pulse.
// TCCR2A |= COM2A1; // redundant repetition
  OCR2A = pwm_min; // set pwm duty cycle
  sei();
} // end setup_PWM()


void set_PWM(){
  // ### if batt_state== charging {do P&O}
  // ### else {adj for batt voltage}
//  analogWrite(PWM_pin, pwm_min);
  OCR2A = pwm_min; // set pwm duty cycle  
} // end set_PWM()

What is the load the pin is driving?

If you don't use any of the registers directly, and use the Arduino function analogWrite() does that work as expected?

Do you have a scope shot if what you are getting?

thanks for the reply.

originally pin3 was unloaded and just seemed to be picking up a bit of noise during the bursts of writing on USB : Serialprint()

Now 6k8 to gnd , no signal.

replaced setup_PWM() with simpler arduino lib calls:

byte const PWM_pin=3;  // ~ = 3.5.6.9.10.11 ; 10.11 on SD ;  Pins 11 and 3: controlled by timer2
byte const pwm_min=128; // ### req min=4 ;  test 128= sqr
byte const pwm_max=250; // min, max ensure bootstrap still charges.

void setup_PWM_stdcalls(){
  pinMode(PWM_pin, OUTPUT);
  analogWrite(PWM_pin, pwm_min);
 // leaves wrong freq and affects pin11 : SPI for SD
 // are interrupts set? 
}

Still no output on pin3.

:?

How about the rest of your code?

OK, progress.

I have the PWM signal on pin three and have control over freq and duty cycle.

void setup_PWM_stdcalls(){
  pinMode(PWM_pin, OUTPUT);
  analogWrite(PWM_pin, pwm_min);
 // leaves wrong freq and affects pin11 : SPI for SD
 // are interrupts set? 
   TCCR2B |=  0b010;  //  div 8 = 2MHz clk -> 8kHz PMW
  OCR2B = pwm_min; // set pwm duty cycle  

}

Note OCR2B , not OCR2A as before.

Have I just got A and B (3,11) the wrong way around ?

referring to the schematic I find arduino "pin 3" is pin 4 on IDL which connects to pin 5 on the atmel chip and is labelled as PD3 (INT1) aka PCINT19 / OC2B.

No friggin wonder I'm getting some of this back to front !

Finally, I've got this doing what I want:

void setup_PWM(){
  pinMode(PWM_pin, OUTPUT);
  OCR2B = pwm_min; // set pwm duty cycle  
  TCCR2A = 0b01;  // WGM2[1,0] 
//  TCCR2A |=  0b10000000;  // COM2A[1,0]= TCCR2A[7,6]  = 10 Clear OC2A on Compare Match, set OC2A at BOTTOM
  TCCR2A |=  0b00100000;  // COM2B[1,0]= TCCR2A[5,4]  = 10 Clear OC2B on Compare Match, set OC2B at BOTTOM
  TCCR2B =  0b0010;  // WGM2[2]=0 + div 8 = 2MHz clk -> 8kHz PMW
// TIMSK2[7..3] reserved  [2..0]= compB compA toie
  TIMSK2=0b101;
}

Thanks for the help. Gently pointing me to the need to figure it out one step at a time did it. Probably burned up about half my ration of flash re-writes on this chip though :frowning:

Now back to my circuit. :slight_smile:

Oops. I cried victory a bit too soon there.

The PWM is working as intended but as soon as I try to hook in an interrupt it blows out Serial printing via USB.

I've spend hours pouring over arduino code and get the impression I'm just getting bogged down in unnecessary detail.

void TIMER2_OVF_vect (void) {
  // if pwd_change
  //   analogWrite(PWM_pin, new_pwm_value); // this is calculated in P&O funct. ###
  // lo off ; pulse(Hin);
  reti();
} // end TIM2 overflow ISR

void setup_PWM_stdcalls(){
  pinMode(PWM_pin, OUTPUT);
  analogWrite(PWM_pin, pwm_min);


// TIMSK2[7..3] reserved  [2..0]= compB compA toie
  TIMSK2 |= 0b01;
} // end sample_cells

If I remove the line setting TIMSK2, I get the PWM signal on arduino pin3 and Serial.print() works. Fine.

With that line, I still get the PWM but the serial communications breaks and the serial device disappears on the linux host PC.

Now unless I'm mistaken, that empty ISR should only cost about 0.5 us and should not be affecting SREG , so what is breaking Serial and USB ?

      40:       0c 94 c5 03     jmp     0x78a   ; 0x78a <__vector_16>
      44:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      48:       0c 94 ec 07     jmp     0xfd8   ; 0xfd8 <__vector_18>
      4c:       0c 94 33 08     jmp     0x1066  ; 0x1066 <__vector_19>
0x000E jmp TIM2_COMPA ; Timer2 Compare A Handler
0x0010 jmp TIM2_COMPB ; Timer2 Compare B Handler
0x0012 jmp TIM2_OVF ; Timer2 Overflow Handler
0x0014 jmp TIM1_CAPT ; Timer1 Capture Handler
0x0016 jmp TIM1_COMPA ; Timer1 Compare A Handler
0x0018 jmp TIM1_COMPB ; Timer1 Compare B Handler
0x001A jmp TIM1_OVF ; Timer1 Overflow Handler

Having set out several times today to find out what interrupt USB write is using I have not managed to find which onion ring of the avr source code hides this information.

Since I removed the Tone lib in case that was getting pulled in, I'm now guessing it may be USB using Timer2 !

Can someone confirm/refute that guess and tell me where I can find that information?

Serial.print() is interrupt based.

ardnut:
Now unless I'm mistaken, that empty ISR should only cost about 0.5 us and should not be affecting SREG , so what is breaking Serial and USB ?

You are mistaken, I believe. According to my calculations here:

An ISR using the ISR define will take you 2.625 uS to execute, plus whatever the code itself does.

That's a lot more than 0.5 uS.

If you want PWM output, I don't see why you are using an ISR anyway. The hardware will do PWM output, regardless of what the rest of the code is doing:

void TIMER2_OVF_vect (void) {
  // if pwd_change
  //   analogWrite(PWM_pin, new_pwm_value); // this is calculated in P&O funct. ###
  // lo off ; pulse(Hin);
  reti();
} // end TIM2 overflow ISR

This looks wrong. You should be using the ISR keyword. The compiler will generate a function preamble and postamble.

Thanks Nick, I had seen that page with your calcs., very informative. I was in the process of composing another post here where I used you figures. I don't think that's the key issue here though.

I'm trying to move away from using a lot of these arduino macros and library calls which just add another level of complexity. The main problem is the code base is so convoluted it's damn near unreadable now. the ISR fleet is a good example. There's about three screenfulls of #defines #ifdefs and non compiled doxygen code examples. The ISR macro comes down to about one line , it just takes about half an hour to parse all the mumbo jumbo to find that out.

Disassembly of section .text:

00000000 <__vectors>:
       0:       0c 94 b9 00     jmp     0x172   ; 0x172 <__ctors_end>
       4:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
       8:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
       c:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      10:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      14:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      18:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      1c:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      20:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      24:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      28:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      2c:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      30:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      34:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      38:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      3c:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      40:       0c 94 60 03     jmp     0x6c0   ; 0x6c0 <__vector_16>
      44:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      48:       0c 94 30 05     jmp     0xa60   ; 0xa60 <__vector_18>
      4c:       0c 94 77 05     jmp     0xaee   ; 0xaee <__vector_19>
      50:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      54:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      58:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      5c:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      60:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
      64:       0c 94 e1 00     jmp     0x1c2   ; 0x1c2 <__bad_interrupt>
#define TIMER0_OVF_vect   _VECTOR(16)  /* Timer/Couner0 Overflow */
#define SPI_STC_vect      _VECTOR(17)  /* SPI Serial Transfer Complete */
#define USART_RX_vect     _VECTOR(18)  /* USART Rx Complete */
#define USART_UDRE_vect   _VECTOR(19)  /* USART, Data Register Empty */
#define USART_TX_vect     _VECTOR(20)  /* USART Tx Complete */
#define ADC_vect          _VECTOR(21)  /* ADC Conversion Complete */

It is quite possible that there is something wrong with that interrupt declaration since I don't see the vector in the vector table in my disassembly. Like I said the code is unreadable, so I may have parsed it wrongly.

If you are able to correct it that would be very helpful.

I do actually want to add some action to the ISR, I just reduced it to an empty one to pin down this bug. I have the hardware doing the PWM as required. My problem is it's blowing out the USB serial link when I enable that interrupt:

TIMSK2 |= 0b01;

I've tested using ISR macro , same thing.

That's the problem I need to fix.

Thanks.

Why do you need an ISR?

If you are going to use one, this is the wrong way to declare it:

void TIMER2_OVF_vect (void) {
  // if pwd_change
  //   analogWrite(PWM_pin, new_pwm_value); // this is calculated in P&O funct. ###
  // lo off ; pulse(Hin);
  reti();
} // end TIM2 overflow ISR

Thanks, re-read my last post, I've made some edits, you may have missed . ie ISR does give a vector but problem persists.

this is the wrong way to declare it:

If you can correct that declaration it would be great. How should it be done without using ISR macro?

What is the objection to the ISR macro? I repeat my question:

Why do you need an ISR?

What is the objection to the ISR macro?

As I detailed above, I find the code base so convoluted it takes a huge amount of effort just to read what it's doing. When you have a bug you need to be able read the code with the problem in order to fix it. If I'm calling code I can't read and hence don't know what it really does, I'm not going to be able to fix any problems. That may work for blinking LEDs but I'm trying to do something a bit more complicated.

If ISR all boils down to one line , I'd rather code the one line directly so that I can read it and know what it does. Same for analogWrite et al : I need to write some tight code here. I don't need to set the port to output every time I write to it. If I need to write a byte to a port I'd rather do just that.

Why do you need an ISR?

I already replied to that, I will need to add some code to the ISR. There is more to this project than outputting a fixed PWM. Some code may go in Timer2_ovl, or in ADC_vect.

ADC input will determine adjustments to duty cycle (and possibly period). If I need to preserve the basic pulse cycle I will need to make that change in OVL. If I can accept skips it may be better in ADC_vect.

Until I can restore minimal debugging interaction via serial I'm pretty much buggered from going anywhere unless I move to jtag or some other interface for debugging. But that's just walking away from the current problem and taking on more issues.

I need find out why a minimal ISR breaks USB serial. If it turns out to be expected and I know why and how , I can decide how to deal with it.

For the moment it's a bug that I don't understand. It's a blocker.

ardnut:
If ISR all boils down to one line , I'd rather code the one line directly so that I can read it and know what it does.

But, you haven't.

#  define ISR(vector, ...)            \
    void vector (void) __attribute__ ((signal,__INTR_ATTRS)) __VA_ARGS__; \
    void vector (void)
#endif

You had:

void TIMER2_OVF_vect (void) {

Where is " attribute ((signal,__INTR_ATTRS))"?

You are overthinking this big-time. People who know what they are doing are making these macros for you. It is not in your interests to dig into the code, replace them with something different, and then complain that it doesn't work.

I already replied to that, I will need to add some code to the ISR.

Why?

PWM on arduino UNO pin3 breaking USB output

Look, the Uno doesn't have USB output. It has serial output.

replace them with something different, and then complain that it doesn't work.

I already said the same thing happens using the ISR macro, so that's beside the point. I am not "complaining" that I have a bug , I'm complaining that the code is nearly illegible, which is impeding me from fixing my bug.

There is plenty of stuff in this code that shows that the naive assumption that "these guys know what they're doing" is not always true. That is the assumption that I started with. In view of the age of this platform and maturity of the project. But the more I look, the more I learn that the quality can not always be counted on.

Hence my desire to strip this down to simple compiler code rather than spend days digging through heaps of obtuse and opaque library code wrapping trivial IO.

I don't want to spend days programming the UART or setting up SPI to an SD card. Libs are great for that sort of thing if they are reliable. The rest I'd rather do 'by hand'.

Quote
I already replied to that, I will need to add some code to the ISR.

Why?

What what? Why don't you quote the text directly after what you quote here where I explain in detail why?

What are you trying to say? There's no bug because I don't need interrupts, anyway?

Well, after being bedevilled by this bullshit for several days it has now evaporated. :stuck_out_tongue_closed_eyes:

I decided to start from scratch with the fading led example and copied each bit of code across in tiny steps building and testing at each step. From time to time I swapped back to make sure the buggy one was still being buggy. It was.

The new program kept working as I gradually got it up to the same state as the problematic one. And the damned thing worked!

The only explanation I can think of is that I included the SDfatlib, which I think I did update since I initially installed it. It was during a call to Pgmprintln() from that lib that it would hang when I was setting the interrupts. And that probably got installed before I added Nick's fix for malloc in wiring.c

It may be that something, somewhere was not getting rebuilt, so something stale was getting linked in.

That's the only logical explanation I can think of , otherwise it must have been the devil sitting behind me jabbing a finger up my arse and I was so busy with the code I did not notice.

Phew! Now finally I can get on with my project.