AVR ATmega2560 Timer1 Mode10 expecting PWM on PB7 pin but nothing!

Go To Last Post
21 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
#ifndef F_CPU
	#define F_CPU 16000000L // Specify oscillator frequency
#endif

#include <avr/io.h>
#include <util/delay.h>
#define myPWM_TMODE 10
 void myPWM_init()
 {
		 ICR1=20000;
		 DDRB|=(1<<7);
		 TCCR1A=((myPWM_TMODE & 0x03)<<WGM10)|(1<<COM1C0);
		 OCR1C=(ICR1>>1)-1;		//50% dutycycle
		 TCCR1B=(((myPWM_TMODE & 0x0C)>>2)<<WGM12)|(1<<CS11)|(0<<CS10);		//pres scaler 0x04=256 0x03=64	//0x02=8	//0x01=1
 }
 
int main(void)
{	
	myPWM_init();		
    while(1)
    {
		OCR1C=9999;
    }
}

I am expecting 50Hz Freq on PB7/OC1C pin but nothing happens. I am doing it for servo motor, after programming LED on arduino mega 2560 doesnt blink at all i.e. L.

This topic has a solution.

AVR Rocks but can be Bricked too smiley

Last Edited: Tue. Aug 15, 2017 - 07:46 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
 TCCR1A=((myPWM_TMODE & 0x03)<<WGM10)|(1<<COM1C0);

I think you want COM1C1 ?

 

[when doing sanity-check work, it helps my hairline to do Fast  PWM first]

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Tried this but , Arduino mega 2560 pin 13 is has internal LED which is not blinking.
 

AVR Rocks but can be Bricked too smiley

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

OMG! actually that code works but it was funny to know that led was blinking so fast :) i reduced freq 25Hz and now its blinking i can see! thanks.

 

#ifndef F_CPU
    #define F_CPU 16000000L // Specify oscillator frequency
#endif

#include <avr/io.h>
#include <util/delay.h>
 #define myPWM_TMODE 10
 void myPWM_init()
 {
        //COM1A1 COM1A0 COM1B1 COM1B0 COM1C1 COM1C0 WGM11 WGM10---- TCCR1A
         //
         ICR1=5000; //25Hz
         //    ICNC1 ICES1 – WGM13 WGM12 CS12 CS11 CS10 TCCR1B
         //PORTB|=(1<<5);
         DDRB|=(1<<7);//PORTB|=(1<<7);
         TCCR1A=((myPWM_TMODE & 0x03)<<WGM10)|(1<<COM1C1)|(0<<COM1C0);
         OCR1C=(ICR1>>1)-1;        //50% dutycycle
         TCCR1B=(((myPWM_TMODE & 0x0C)>>2)<<WGM12)|(1<<CS11)|(1<<CS10);        //pres scaler 0x04=256 0x03=64    //0x02=8    //0x01=1
 }
int main(void)
{    
    myPWM_init();        
    while(1)
    {
               
    }
}


 

 

AVR Rocks but can be Bricked too smiley

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Are you sure that you are loading the correct build results?

 

Perhaps show the code you are using now.

 

Perhaps use a simpler mode such as CTC to see if the LED is actually working the way you think.  Are there other output pins you can experiement with?

 

I Googled for Arduino Mega pin mapping, and the list I found agrees that digital pin 13 is PB7.

 

CodeVision Wizard agrees with your setup...

 

...
// Port B initialization
// Function: Bit7=Out Bit6=In Bit5=In Bit4=In Bit3=In Bit2=In Bit1=In Bit0=In 
DDRB=(1<<DDB7) | (0<<DDB6) | (0<<DDB5) | (0<<DDB4) | (0<<DDB3) | (0<<DDB2) | (0<<DDB1) | (0<<DDB0);
...
// Timer/Counter 1 initialization
// Clock source: System Clock
// Clock value: 2000.000 kHz
// Mode: Ph. correct PWM top=ICR1
// OC1A output: Disconnected
// OC1B output: Disconnected
// OC1C output: Non-Inverted PWM
// Noise Canceler: Off
// Input Capture on Falling Edge
// Timer Period: 20 ms
// Output Pulse(s):
// OC1C Period: 20 ms Width: 9.999 ms
// Timer1 Overflow Interrupt: Off
// Input Capture Interrupt: Off
// Compare A Match Interrupt: Off
// Compare B Match Interrupt: Off
// Compare C Match Interrupt: Off
TCCR1A=(0<<COM1A1) | (0<<COM1A0) | (0<<COM1B1) | (0<<COM1B0) | (1<<COM1C1) | (0<<COM1C0) | (1<<WGM11) | (0<<WGM10);
TCCR1B=(0<<ICNC1) | (0<<ICES1) | (1<<WGM13) | (0<<WGM12) | (0<<CS12) | (1<<CS11) | (0<<CS10);
TCNT1H=0x00;
TCNT1L=0x00;
ICR1H=0x4E;
ICR1L=0x20;
OCR1AH=0x00;
OCR1AL=0x00;
OCR1BH=0x00;
OCR1BL=0x00;
OCR1CH=0x27;
OCR1CL=0x0F;

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
#ifndef F_CPU
	#define F_CPU 16000000L // Specify oscillator frequency
#endif

#include <avr/io.h>
#include <util/delay.h>
 #define myPWM_TMODE 10
 void myPWM_init()
 {
		//COM1A1 COM1A0 COM1B1 COM1B0 COM1C1 COM1C0 WGM11 WGM10---- TCCR1A
		 //
		 ICR1=20000;
		 //	ICNC1 ICES1 – WGM13 WGM12 CS12 CS11 CS10 TCCR1B
		 //PORTB|=(1<<5);
		 DDRB|=(1<<7);//PORTB|=(1<<7);
		 TCCR1A=((myPWM_TMODE & 0x03)<<WGM10)|(0<<COM1C1)|(1<<COM1C0);
		 OCR1C=(ICR1>>1)-1;		//50% dutycycle
		 TCCR1B=(((myPWM_TMODE & 0x0C)>>2)<<WGM12)|(1<<CS11)|(0<<CS10);		//pres scaler 0x04=256 0x03=64	//0x02=8	//0x01=1
 }
int main(void)
{	
	myPWM_init();		
    while(1)
    {	
    }
}

Timer 1 mode 10 ; PWM freq=50Hz with 50% duty cycle; PB7 pin of Arduino Mega 2560. Thanks now its OK.

AVR Rocks but can be Bricked too smiley

This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

engineer.pk wrote:
Thanks now its OK.

???

 

But you have the same issue that I pointed out in #2  ?!?

 

 TCCR1A=((myPWM_TMODE & 0x03)<<WGM10)|(0<<COM1C1)|(1<<COM1C0);

According to the datasheet table, COM1:0 value of 0:1 is a pin disconnected from the timer in that mode.

 

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Tue. Aug 15, 2017 - 07:33 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
TCCR1A=((myPWM_TMODE & 0x03)<<WGM10)|(1<<COM1C1)|(0<<COM1C0);

Works as you pointed out, THanks,

AVR Rocks but can be Bricked too smiley

Last Edited: Tue. Aug 15, 2017 - 07:41 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello everyone,

I am not sure if posting to this thread is as it is marked solved is a good thing to do but as I have the same problem it seemed silly to start another one.

 

I am using a ATMega2560 on a Arduino Mega2560 board to drive a 3 Phase Brushless DC motor with a 20KHz PWM signal from the 3 outputs of Timer1 in mode 10.

Output from A & B on device pins 24 & 25 (Digital Pins 11/12) are fine, however device pin 26 (Digital Pin 13) just sits at 0V.

 

I am aware from section 19 of the device data sheet that pin 26 (Digital Pin 13) shares its PWM output with the A output of Timer 0 and that which one is presented at the pin is dependent on the COMnx bits of the relevant TCCRnx registers. Unless I have been very unfortunate in that both boards/devices I have tried are broken I can only assume the problem must be with my code/device set up, I have cut my code down to be bare minimum to try and isolate the problem although I must be code blind as I still cannot find it, it looks correct to me.

I am coding and uploading using the Arduino IDE and its default fuse setting for the board, can anyone see where I have gone wrong?

 

//Load required Includes - Complete 31/03/2017
#include <avr/wdt.h>    //Include for Watchdog Timer - sgm June 2017

//*** Assign Alias's to Arduino board I/O pin numbers *** - complete 06/06/2017
//MOTOR CONTROL
#define PWM_SPARE 11    //Spare PWM output (OC1A)
#define DCM_PWM 12    //DC Load Machine PWM/Inhibit (OC1B) - rename DC PWM.
#define PWM 13          //PWM source for single BLDCM phase PWM signal / Always high gate for individual BLDCM phase PWM signals (OC1C) 

void setup() {

TCCR0A = TCCR0A & 0x3F;           //Disconnect Timer OCOA output from Digital Pin 13 (PB7) so that it can be used by Timer 1.
TCCR1A = (TCCR1A & 0x00) | 0xAA;  //Ensure TCCR1A is clear then Set for Phase Correct PWM, TOP = ICRn, output = A, B & C (1010 1010)
TCCR1B = (TCCR1B & 0x00) | 0x11;  //Ensure TCCR1B is clear then set for PWM, no Prescaling 0001 0001 - DO NOT CHANGE bit 7:3
TCCR1C = (TCCR1C & 0x00);         //Ensure TRRC1C is clear, TCCR1C not used, just belt & braces code
ICR1 = 0x0190;                    //Use IRCn to Set TOP for a 20KHz PWM Frequency  = 400 ($0190) 

//Set up Baud rate for Serial (USB) coms - Complete
//Serial.begin(19200);

//Set up initial state for Infineon Motor drivers & PWM Signal- Complete 07/06/2017
digitalWrite(PWM_SPARE, LOW);  //No PWM - Output in Tristate condition
digitalWrite(DCM_PWM, LOW);  //No DC Load Machine PWM - Output in Tristate condition
digitalWrite(PWM, LOW);  //No PWM - Output in Tristate condition

//*** Set up I/O status of Infineon Motor driver control & PWM output pins ***
//Set DC Load Machine PWM control pins to Outputs (Digital Pins) - Complete 06/06/2017
pinMode(DCM_PWM, OUTPUT);
pinMode(PWM, OUTPUT);
pinMode(PWM_SPARE, OUTPUT);

//Set Modulation depth (Testing only)
analogWrite(PWM, 100);        //25% Modulation depth
analogWrite(DCM_PWM, 200);    //50% Modulation depth
analogWrite(PWM_SPARE, 300);  //75% Modulation Depth

WDTO_2S;  //2 Second Time out (Watchdog Timer enabled at power up)
}
// END OF SETUP

//=========================
//=== Main Program loop === does not return data (Void) - basically done needs a few tweeks
//=========================
void loop() {

//Reset Watchdog Timer (AVR/wdt.h)
wdt_reset();
//asm volatile("wdr");  //Reset Watchdog Timer using Assembler command - I hope?
}
//=== END OF Main Program Loop ===

Regards,

Stephen

Last Edited: Thu. Oct 19, 2017 - 08:58 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

What follows might not be a solution to your problem, but is at least advice on how to make your code more easily debugged, and more easily read by yourself and all of us here:

 

TCCR1A = (TCCR1A & 0x00) | 0xAA;

Don't over-complicate things. First anything & 0 is 0, so now we have

TCCR1A = 0 | 0xAA;

Next, 0 | something is something, so we're left with

TCCR1A = 0xAA;

Then make things clearer: refrain from using magic numbers (like 0xAA) where you and us need to decipher to 1010 1010 and go back to the data sheet to check out what those bits of TCCR1A are: COM1A1, COM1B1, COM1C1, WGM11. Now the shift-operator comes in handy and we do some bit-wise operations that actually makes for clearer, more easily read code:

 

TCCR1A = (1<<COM1A1) | (1<<COM1B1) | (1<<COM1C1) | (1<<WGM11);

From that code it is instantly clear what bits are set.

 

Likewise for TCCR1B, we get

TCCR1B = (1<<WGM13) | (1<<CS10);

and for TCCR1C the simple

TCCR1A = 0;

 

So we have mode 10 if my reading is correct, i.e. phase correct PWM with TOP in ICR1. Seems just about right.

 

For the COMnA1:0 bits we have the value (binary) 10 which is "clear on compare match". Also OK, it seems.

 

You're never setting OCR1A or OCR1B though. A duty-cycle of 0% is not a far-fetched hypothesis..

 


 

Next, it makes no little sense to me to use hex notation when setting the TOP/ICR1 value. This is perhaps better

ICR1 = 400;

but the real clarity would come from an expression stating explicitly how you got to 200.

 


 

I have not checked that your assumptions of mappings of pins from 2560 pin numbers to Arduino pin numbers are correct.

 

I am a bit suspicious about the comment 

Disconnect Timer OCOA output from Digital Pin 13 (PB7) so that it can be used by Timer 1.

but am too lazy ;-) to check whether Timer/Counter0 and Timer/Counter1 actually interfere with one another in this way.

 


Side notes:

TCCR1B = (TCCR1B & 0x00) | 0x11;  //Ensure TCCR1B is clear then set for PWM, no Prescaling 0001 0001 - DO NOT CHANGE bit 7:3

The comment is wrong. The statement does not in any way guarantee that bit 7:3 is left untouched. The statement always sets bits 7:5 and 3:1 to zero. Bits 4 and 0 are set to one.

"He used to carry his guitar in a gunny sack, or sit beneath the tree by the railroad track. Oh the engineers would see him sitting in the shade, Strumming with the rhythm that the drivers made. People passing by, they would stop and say, "Oh, my, what that little country boy could play!" [Chuck Berry]

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

Last Edited: Thu. Oct 19, 2017 - 10:22 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I am new to C and Arduino having been given the job of improving someone else's code and this is the only issue I have not been able to resolve myself, consequently some of the points raised as to my coding style result from following the previous persons method as that appeared to work and so as far as I knew was the way to do it. I agree that I should have used decimal for the IRC1 value. 

 

I do not see anything wrong with using hex values when setting register bits, particularly if well commented as they save a lot of time and code space over writing commands for individual bits and for 16, 32 & 64 bit register are infinitely easier to use and see which bits are set than using decimal or binary as it allows you handle it in small, simple 4 bit chunks.

 

I did not see a need to show how I determined the value for ICR1 in my cut down code because it is the correct value, resulting in the desired 20KHz PWM from OC1A & OC1B, just not also from OC1C as I would have expected. Thus showing how it was derived is really irrelevant.

 

Device pin26/Arduino Digital Pin 13 is shared by the Timer0A and Timer1C outputs, depending on the COMnx register bit values the out put on that pin will be either OC0A, OC1C or one modulated by the other hence setting the COMnx bits to disable the OC0A output and allow the OC1C output to appear on that pin.

 

As for the comment...

You're never setting OCR1A or OCR1B though. A duty-cycle of 0% is not a far-fetched hypothesis..

I do not think I have to, is that not what the analogWrite commands are doing as they are producing the correct pulse width outputs from Timer1 OC1A & OC1B?. If you meant use them to set TOP I cannot as I am using mode 10 (need PWM from OC1A), thus ICR1 is used to set top. 

 

Perhaps...

TCCR1A = (1<<COM1A1) | (1<<COM1B1) | (1<<COM1C1) | (1<<WGM11);

​makes for clear and readable code for an experienced C programmer but it does not to me. If you want to set a bit then using equals or the assembler bset makes for more readable and easier to understand code as it does what it says on the tin, i.e.

TCCR1A = (COM1A1=1) | (COM1B1=1) | (COM1C1=1) | (WGM11=1);

to use bit shift seems illogical and confusing to me as shifting bits is neither what I want to do or clearly/accurately defines what you are setting a bit.

 

I have taken on board you comments and changed the code/code comments to hopefully make it more readable/understandable, however it may still not be exactly as those here would prefer as I need to be able to understand it quickly should I ever have to come back to it at a later time.

//Load required Includes - Complete 31/03/2017
#include <avr/wdt.h>    //Include for Watchdog Timer - sgm June 2017

//*** Assign Alias's to Arduino board I/O pin numbers *** - complete 06/06/2017
//MOTOR CONTROL
#define PWM_SPARE 11  //Spare PWM output (OC1A)
#define DCM_PWM 12    //DC Load Machine PWM/Inhibit (OC1B) - rename DC PWM.
#define PWM 13        //PWM source for single BLDCM phase PWM signal / Always high gate for individual BLDCM phase PWM signals (OC1C) 

void setup() {

TCCR0A = TCCR0A & 0x3F;  //Set COM0A1 & COM0A0 to 0, disables Timer OCOA output so that only Timer1 output is presented on Digital Pin 13 (PB7)
TCCR1A = 0xAA;           //Set COM1A1, COM1B1, COM1C1 & WGM11 to 1 (1010 1010) = Phase Correct PWM, TOP = ICRn, output = Timer1 A, B & C
TCCR1B = 0x11;           //Set WGM13 & CS10 to 1 for PWM with no Prescaling (0001 0001) - DO NOT CHANGE bits 7:3 of this value, only change bits 2:0 (prescalling)
TCCR1C = 0x00;           //Ensure TRRC1C is clear, TCCR1C not used, just belt & braces code
ICR1 = 400;              //Use IRCn to Set TOP for a 20KHz PWM Frequency  = 400 ($0190) 

//Set up initial state for Infineon Motor drivers & PWM Signal- Complete 07/06/2017
digitalWrite(PWM_SPARE, LOW);  //No PWM - Output in Tristate condition
digitalWrite(DCM_PWM, LOW);    //No DC Load Machine PWM - Output in Tristate condition
digitalWrite(PWM, LOW);        //No PWM - Output in Tristate condition

//*** Set up I/O status of Infineon Motor driver control & PWM output pins ***
pinMode(PWM_SPARE, OUTPUT);  //Set pin to Output
pinMode(DCM_PWM, OUTPUT);    //Set pin to Output
pinMode(PWM, OUTPUT);        //Set pin to Output

//Set Modulation depth (Testing only)
analogWrite(PWM_SPARE, 300);  //75% Pulse width on Timer1A (OC1A)
analogWrite(DCM_PWM, 200);    //50% Pulse width on Timer1B (OC1B)
analogWrite(PWM, 100);        //25% Pulse width on Timer1C (OC1C)

WDTO_2S;  //2 Second Time out (Watchdog Timer enabled at power up)
}
// END OF SETUP

//=========================
//=== Main Program loop === does not return data (Void) - basically done needs a few tweeks
//=========================
void loop() {

//Reset Watchdog Timer (AVR/wdt.h)
wdt_reset();
//asm volatile("wdr");  //Reset Watchdog Timer using Assembler command - I hope?
}
//=== END OF Main Program Loop ===

I have heard from another source that the problem could well be with the Arduino runtime and not my code as apparently it is unaware of OC1C and so any attempts to use it either fail or result in OCOA being presented to the pin. Can anyone confirm that and if so is there a know work around?

 

Regards,

Stephen

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

sgm23 wrote:
I do not see anything wrong with using hex values when setting register bits, particularly if well commented
that may be true if the comments ALWAYS match what the code is doing. Too often we see threads here where the problem turns out to be that the comment does not match what the statement does (last minute "tweaks" etc). So common practice here is to not trust the comments.

 

Now if you read:

TCCR1A = (1<<COM1A1) | (1<<COM1B1) | (1<<COM1C1) | (1<<WGM11);

it is unequivocal what that is achieving. For:

TCCR1A = 0xAA;

readers here have no option but to go and dig out the datasheet for this device and hand decode the magic number to see if the value used and the comment really do match.

 

Of course this cannot catch things like:

TCCR1A = (1<<TXEN) | (1<<COM1B1) | (1<<ADIF) | (1<<WGM11);

where the user is setting bits that aren't even in that register. (a common example being that WGM bits tend to have 0 and 1 in the A register and 2 and 3 in the B register).

 

But you are more likely to get help here if you present code that can easily be read by other AVR programmers.

 

That also comes back to you in 18 months time and YOU are the "new person" who is unfamiliar with the code (everyone forgets!) - you read 0xAA - now what does that set again? Can I trust the possibly out of date comment alongside?

Last Edited: Fri. Oct 20, 2017 - 02:23 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

sgm23 wrote:
If you want to set a bit then using equals or the assembler bset makes for more readable and easier to understand code as it does what it says on the tin, i.e. TCCR1A = (COM1A1=1) | (COM1B1=1) | (COM1C1=1) | (WGM11=1); ...

 

Are you showing this as an example of the type of syntax that you would prefer, or one that you use?

 

If you try to use that, realize that e.g. COM1A1 is nothing more than "7", and "(7=1)" is simply nonsense.

#define    COM1A1          7       // Compare Output Mode 1A, bit 1

Mentioning BSET in this context is not of much help, as that only pertains to SREG.  More pertinent would be SBI with takes the bit number.  And the default/typical C header will usually follow the above as shown with the bit number.

 

If you want to take the assembler thought one step further, then examine e.g. SBR -- which takes a bit mask just as in the TCCR1A line in C.

 

You haven't really said which AVR model you are dealing with.  About a decade ago Atmel decided to apply the new-and-improved "universal" I/O register map to Mega models.  To a large extent that has made SBI and similar of little use in peripheral work, as they are out of reach.

 

There are others that share your opinion.  [It's not that I don't share it; it's just that I'll tend to use the toolchain-provided chip-include...]  A former colleague would create a complete set of bit masks in an "auxiliary" header file, e.g.

//-----------------------------------------------------------------------------
// USART Control and Status Register C  (URSEL=1)                        (UCSRC)
#define BIT_UCSRC_URSEL        (1 << 7)    // Register Select
#define BIT_UCSRC_UMSEL        (1 << 6)    // USART Mode Select
#define BIT_UCSRC_UPM1        (1 << 5)    // USART Parity Mode Enable (0=None)
#define BIT_UCSRC_UPM0        (1 << 4)    // USART Parity Type (0=Even, 1=Odd)
#define BIT_UCSRC_USBS        (1 << 3)    // Stop Bit Select
#define BIT_UCSRC_UCPOL        (1 << 0)    // Clock Polarity

 

Now, what was your question again? ;)  I applaud your assignment to timer-control registers in your setup() -- it isn't just belt-and-braces, but rather Arduino sets up timers in a default configuration.  You might need to blow away even more of timer0 config, keeping in mind which timer Arduino uses for timekeeping on that particular model.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I am looking into the Arduino sources to see how AnalogWrite(...) does things. I am not at the bottom of it yet. Need to sort out some conditional compilation stuff and mappings between 2560 pins and Arduinos pin numbering scheme w.r.t the specifics of Arduino2560.

 

At the moment it seems to me that the Arduino sources does not map Arduino pin 13 so that 

 

AnalogWrite(13, value);

actually writes to OCR1C. If I'm correct (which I am not sure of yet) then for any value less than 128 the pin will be set to a constant/steady low (i.e. 0% duty cycle) and for any other value it will be set constant/steady high (100%).

 

Need food. Back later..

"He used to carry his guitar in a gunny sack, or sit beneath the tree by the railroad track. Oh the engineers would see him sitting in the shade, Strumming with the rhythm that the drivers made. People passing by, they would stop and say, "Oh, my, what that little country boy could play!" [Chuck Berry]

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

First thing that is clear: analogWrite(pin, value) is designed to take values from 0 to 255, so this won't work:

 

analogWrite(PWM_SPARE, 300);

After cutting out a lot of irrelevant stuff from analogWrite() and this is what I'm left with:

 

void analogWrite(uint8_t pin, int val)
{
	pinMode(pin, OUTPUT);
	if (val == 0)
	{
		digitalWrite(pin, LOW);
	}
	else if (val == 255)
	{
		digitalWrite(pin, HIGH);
	}
	else
	{
		switch(digitalPinToTimer(pin))
		{
			case TIMER1A:
				// connect pwm to pin on timer 1, channel A
				sbi(TCCR1A, COM1A1);
				OCR1A = val; // set pwm duty
				break;

			case TIMER1B:
				// connect pwm to pin on timer 1, channel B
				sbi(TCCR1A, COM1B1);
				OCR1B = val; // set pwm duty
				break;

			case TIMER1C:
				// connect pwm to pin on timer 1, channel B
				sbi(TCCR1A, COM1C1);
				OCR1C = val; // set pwm duty
				break;
			case NOT_ON_TIMER:
			default:
				if (val < 128) {
					digitalWrite(pin, LOW);
				} else {
					digitalWrite(pin, HIGH);
				}
		}
	}
}

As you can see, before any mapping from Arduino pin number to timer register takes place there are special handling of val being 0 or 255. It is quite clear that analogWrite relies on a timer where the TOP value is 255 (not 400). This alone is enough to stop using analogWrite and write to OCR1A/B/C explicitly.

 


 

As for the specific problem with the pin 13 PWM output I'm still digging into 

digitalPinToTimer(pin)

My suspicion is that it does not handle pin 13 and simply returns NOT_IN_TIMER - rather than the TIMER1C that is needed for a working on pin 13 (a.k.a OC1C).

 


 

Even without this pin 13 problem investigated to the end, the fact that analogWrite() is designed for values between 0 and 255 is enough to abandon the use of it and write to OCR1A/B/C explicitly instead.

 


 

ADDENDUM

 

From the documentation of analogWrite() :

 

Parameters

pin: the pin to write to.

value: the duty cycle: between 0 (always off) and 255 (always on).

[my emphasis]

 

"He used to carry his guitar in a gunny sack, or sit beneath the tree by the railroad track. Oh the engineers would see him sitting in the shade, Strumming with the rhythm that the drivers made. People passing by, they would stop and say, "Oh, my, what that little country boy could play!" [Chuck Berry]

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

Last Edited: Fri. Oct 20, 2017 - 05:15 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

What follows requires you to study the Arduino framework sources. Without that the below might not make much sense. If you opt out of trying to understand what follows, skip directly to Short Version at the end of this post.

 

Take a look at my cut-down version of analogWrite() above. Notice that it maps from an Arduino pin number to an enum value representing the timer register that should be manipulated:

 

void analogWrite(uint8_t pin, int val)
{
    .
    .
    .
        switch(digitalPinToTimer(pin))
        {
            case TIMER1A:
                .
                .
                .
                break;

            case TIMER1B:
                .
                .
                .
                break;

            .
            .
            .
       }
    }
}

Next, let's look at what digitalPinToTimerPin does. It's in arduino.h :

 

#define digitalPinToTimer(P) ( pgm_read_byte( digital_pin_to_timer_PGM + (P) ) )

So, it's a macro, that does a "lookup" in a table that is in flash memory (hence the pgm_read_byte). The table is named digital_pin_to_timer_PGM, and the parameter P is the offset into that table. The table is defined in pins_arduino.h:

 

const uint8_t PROGMEM digital_pin_to_timer_PGM[] = {
	// TIMERS
	// -------------------------------------------
	NOT_ON_TIMER	, // PE 0 ** 0 ** USART0_RX
	NOT_ON_TIMER	, // PE 1 ** 1 ** USART0_TX
	TIMER3B	, // PE 4 ** 2 ** PWM2
	TIMER3C	, // PE 5 ** 3 ** PWM3
	TIMER0B	, // PG 5 ** 4 ** PWM4
	TIMER3A	, // PE 3 ** 5 ** PWM5
	TIMER4A	, // PH 3 ** 6 ** PWM6
	TIMER4B	, // PH 4 ** 7 ** PWM7
	TIMER4C	, // PH 5 ** 8 ** PWM8
	TIMER2B	, // PH 6 ** 9 ** PWM9
	TIMER2A	, // PB 4 ** 10 ** PWM10
	TIMER1A	, // PB 5 ** 11 ** PWM11
	TIMER1B	, // PB 6 ** 12 ** PWM12
	TIMER0A	, // PB 7 ** 13 ** PWM13
	NOT_ON_TIMER	, // PJ 1 ** 14 ** USART3_TX
	NOT_ON_TIMER	, // PJ 0 ** 15 ** USART3_RX
	NOT_ON_TIMER	, // PH 1 ** 16 ** USART2_TX
	NOT_ON_TIMER	, // PH 0 ** 17 ** USART2_RX
	NOT_ON_TIMER	, // PD 3 ** 18 ** USART1_TX
	NOT_ON_TIMER	, // PD 2 ** 19 ** USART1_RX
	NOT_ON_TIMER	, // PD 1 ** 20 ** I2C_SDA
	NOT_ON_TIMER	, // PD 0 ** 21 ** I2C_SCL
	NOT_ON_TIMER	, // PA 0 ** 22 ** D22
	NOT_ON_TIMER	, // PA 1 ** 23 ** D23
	NOT_ON_TIMER	, // PA 2 ** 24 ** D24
	NOT_ON_TIMER	, // PA 3 ** 25 ** D25
	NOT_ON_TIMER	, // PA 4 ** 26 ** D26
	NOT_ON_TIMER	, // PA 5 ** 27 ** D27
	NOT_ON_TIMER	, // PA 6 ** 28 ** D28
	NOT_ON_TIMER	, // PA 7 ** 29 ** D29
	NOT_ON_TIMER	, // PC 7 ** 30 ** D30
	NOT_ON_TIMER	, // PC 6 ** 31 ** D31
	NOT_ON_TIMER	, // PC 5 ** 32 ** D32
	NOT_ON_TIMER	, // PC 4 ** 33 ** D33
	NOT_ON_TIMER	, // PC 3 ** 34 ** D34
	NOT_ON_TIMER	, // PC 2 ** 35 ** D35
	NOT_ON_TIMER	, // PC 1 ** 36 ** D36
	NOT_ON_TIMER	, // PC 0 ** 37 ** D37
	NOT_ON_TIMER	, // PD 7 ** 38 ** D38
	NOT_ON_TIMER	, // PG 2 ** 39 ** D39
	NOT_ON_TIMER	, // PG 1 ** 40 ** D40
	NOT_ON_TIMER	, // PG 0 ** 41 ** D41
	NOT_ON_TIMER	, // PL 7 ** 42 ** D42
	NOT_ON_TIMER	, // PL 6 ** 43 ** D43
	TIMER5C	, // PL 5 ** 44 ** D44
	TIMER5B	, // PL 4 ** 45 ** D45
	TIMER5A	, // PL 3 ** 46 ** D46
	NOT_ON_TIMER	, // PL 2 ** 47 ** D47
	NOT_ON_TIMER	, // PL 1 ** 48 ** D48
	NOT_ON_TIMER	, // PL 0 ** 49 ** D49
	NOT_ON_TIMER	, // PB 3 ** 50 ** SPI_MISO
	NOT_ON_TIMER	, // PB 2 ** 51 ** SPI_MOSI
	NOT_ON_TIMER	, // PB 1 ** 52 ** SPI_SCK
	NOT_ON_TIMER	, // PB 0 ** 53 ** SPI_SS
	NOT_ON_TIMER	, // PF 0 ** 54 ** A0
	NOT_ON_TIMER	, // PF 1 ** 55 ** A1
	NOT_ON_TIMER	, // PF 2 ** 56 ** A2
	NOT_ON_TIMER	, // PF 3 ** 57 ** A3
	NOT_ON_TIMER	, // PF 4 ** 58 ** A4
	NOT_ON_TIMER	, // PF 5 ** 59 ** A5
	NOT_ON_TIMER	, // PF 6 ** 60 ** A6
	NOT_ON_TIMER	, // PF 7 ** 61 ** A7
	NOT_ON_TIMER	, // PK 0 ** 62 ** A8
	NOT_ON_TIMER	, // PK 1 ** 63 ** A9
	NOT_ON_TIMER	, // PK 2 ** 64 ** A10
	NOT_ON_TIMER	, // PK 3 ** 65 ** A11
	NOT_ON_TIMER	, // PK 4 ** 66 ** A12
	NOT_ON_TIMER	, // PK 5 ** 67 ** A13
	NOT_ON_TIMER	, // PK 6 ** 68 ** A14
	NOT_ON_TIMER	, // PK 7 ** 69 ** A15
};

Notice the entries for Arduino pins 11 and 12:

	TIMER1A	, // PB 5 ** 11 ** PWM11
	TIMER1B	, // PB 6 ** 12 ** PWM12

They map to the identifiers TIMER1A and TIMER1B respectively. Going back to analogWrite this means that using pin number 11 or 12 will write to OCR1A and OCR1B respectively.

 

Now, what about pin 13? Well, the entry in the lookup table is

	TIMER0A	, // PB 7 ** 13 ** PWM13

Notice the TIMER0A! That does not seem to be what you want.. Let's just verify by looking into analogWrite() one last time:

void analogWrite(uint8_t pin, int val)
{

        .
        .
        .

		switch(digitalPinToTimer(pin))
		{
			case TIMER0A:
				// connect pwm to pin on timer 0
				sbi(TCCR0, COM00);
				OCR0 = val; // set pwm duty
				break;

        .
        .
        .

So now we can confirm the whole "chain of events" when using 13 as the pin number when calling analogWrite():

 

1. User sketch calls analogWrite(13, value)

2. analogWrite calls digitalPinToTimer(13)

3. digitalPinToTimer returns TIMER0A

4. analogWrite "decides" that this means to write value to OCR0

 

Conclusion: AnalogWrite, and it's supporting functions, does not support PWM on third channel of Timer1. Whether this is by design or a bug is not clear to me. (What is clear is that nowhere in the Arduino sources is there any mapping from Arduino pin 13 to the symbol TIMER1C.)

 

Remedy is to skip using analogWrite(), and explicitly write to the timer registers - in this case OCR1C. 

 

Short Version

The Arduino framework does not support PWM (or "analog output" if you wish) on third channel of Timer 1 (i.e. on ATmega2560 pin OC1C). Also, note what I concluded in an earlier post: analogWrite is designed for the val parameter to be in the range 0..255. It will set 100%  duty cycle if you pass the value 255 to it.

 

Remedy: Write explicitly to the OCR1A/B/C registers instead. 

 

It will not be much code.

 

Sketchy code

 

Write a function taking the Arduino pin number and a timer match value as parameters. This is sketchy and untested:

 

void setPwm(uint8_t pin, int val) {
    if (val <= 0) {
        digitalWrite(pin, 0);
    }
    else if (val >= 400) {
        digitalWrite(pin, 1);
    }
    else {
        switch (pin) {
            case 11:
                OCR1A = val;
                break;
            case 12:
                OCR1B = val;
                break;
            case 13:
                OCR1C = val;
                break;
        }
    }
}

I've chosen to mimic the handling of values 0 and the "max value" explicitly, just as Arduino does - but i loathe the == tests they use (for obvious reasons?) so sketched using <= and >= instead.

 

HTH!

"He used to carry his guitar in a gunny sack, or sit beneath the tree by the railroad track. Oh the engineers would see him sitting in the shade, Strumming with the rhythm that the drivers made. People passing by, they would stop and say, "Oh, my, what that little country boy could play!" [Chuck Berry]

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

Last Edited: Fri. Oct 20, 2017 - 06:20 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi JohanEkdahl,

thanks for spending some time on this.

 

JohanEkdahl wrote:

First thing that is clear: analogWrite(pin, value) is designed to take values from 0 to 255, so this won't work:

 

analogWrite(PWM_SPARE, 300);

 

You may be correct although I do not think so, it is possible the default value set for TOP is 255 in which case you would be correct and a value of 300 would not work. However, the data sheet for the ATmega2560 shows the maximum count for TOP as, and I quote 0xFFFF which equals 65535 for those who don't like magic numbers. Thus with TOP in ICR1 set to 400 to get a 20KHz output (derived from the calculation in the data sheet) an analogue write value of 300 is acceptable being less than TOP and gives a mark space ratio of 3:1 (75% pulse width) which is as it should be.

 

JohanEkdahl wrote:

 

So, it's a macro, that does a "lookup" in a table that is in flash memory (hence the pgm_read_byte). The table is named digital_pin_to_timer_PGM, and the parameter P is the offset into that table. The table is defined in pins_arduino.h:

 

Notice the entries for Arduino pins 11 and 12:

	TIMER1A	, // PB 5 ** 11 ** PWM11
	TIMER1B	, // PB 6 ** 12 ** PWM12

They map to the identifiers TIMER1A and TIMER1B respectively. Going back to analogWrite this means that using pin number 11 or 12 will write to OCR1A and OCR1B respectively.

 

Now, what about pin 13? Well, the entry in the lookup table is

	TIMER0A	, // PB 7 ** 13 ** PWM13

Notice the TIMER0A! That does not seem to be what you want.. 

You are correct, the problem is solved by adding a new line

TIMER1C , // PB 7 ** 13 ** PWM13

There was nothing wrong with my code - it was all caused by  this omission in the file!

Now, provided you have disabled the TIMER0A output (OCOA) by setting COM0A1 & COM0A0 bits of the TCCR0A register to 0 and set either one or both of the COM1C1 & COM1C0 bits of the TCCR1A register to 1 (as applicable) you now get the PWM output from TIMER1C (OC1C) on pin26 of the ATmega2560/Digital pin 13 of the Arduino Mega2560 board which is what you should be able to do.

Last Edited: Mon. Oct 23, 2017 - 12:06 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You may be correct although I do not think so, it is possible the default value set for TOP is 255 in which case you would be correct and a value of 300 would not work. However, the data sheet for the ATmega2560 shows the maximum count for TOP as, and I quote 0xFFFF which equals 65535 for those who don't like magic numbers.

You're m issing the point. It does not have anything to do with what the hardware timer has as its TOP value. It has everything to do with the code in analogWrite treating the input value 255 in a special way. It sets the PWM pin to constant on, using special code. Just read the source for it. Either by going to the Arduino sources, or look at my cut down "quotes" of it above.

 

The effect will be this:

  • When you call analogWrite(..., 254) you will get a duty cycle of 254/400, i.e. 63.5%
  • When you call analogWrite(..., 255) you will get a dudty cycle of 100% (becaause analogWrite has special coded for this case)
  • When you call analogWrite(..., 256) you will get a duty cycle of 256/400, i.e. 64%

I take it this is not what you want?

 

If you can't see this after looking at the source for analogWrite then please come back and I'll try another way of pointing this out to you.

 

 

You are correct, the problem is solved by adding a new line

TIMER1C , // PB 7 ** 13 ** PWM13

That is not a good solution. Whenever you update the Arduino environment to a new version that "patch" will be lost. If your sketch is moved to another machine then that "patch" will be missing.

 

You are of-course absolutely free to do whatever you like, but if you want this to work in the long  run, and without "glitches" on your PWM signals, then re-read the thread and think about things. Worst of all, since the entries are in context of their relative position in the table, you've now messed up everything following the entry you added.

 

I still claim that

i) analogWrite() is coded specifically for a TOP value of 255, you are pulling the rug under it by overriding the TOP value that the Arduino framework uses and assumes

ii) analogWrite is not suitable for your case, and you should write to the OCR1x registers explicitly

and now add

iii) your "patch" to the Arduino "system" file will break Arduino functionality.

 

Good luck. Anyway.. ;-)

 

 

"He used to carry his guitar in a gunny sack, or sit beneath the tree by the railroad track. Oh the engineers would see him sitting in the shade, Strumming with the rhythm that the drivers made. People passing by, they would stop and say, "Oh, my, what that little country boy could play!" [Chuck Berry]

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

Last Edited: Mon. Oct 23, 2017 - 01:08 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The documentation for analogWrite() is unequivocal:

 

That clearly says "between 0 and 255".  So all bets were off as soon as you wrote:

analogWrite(PWM, 100);        //25% Modulation depth
analogWrite(DCM_PWM, 200);    //50% Modulation depth
analogWrite(PWM_SPARE, 300);  //75% Modulation Depth

Those comments do not describe what you will get (which seems to imply a 0..399 range). 100/255 will give 39% duty not 25%, 200/255 will give a 78% duty not 50% and 300 is clearly "undefined" given that the manual told you it should be 0..255.

 

Now elsewhere Arduino has a function to map ranges of values:

 

https://www.arduino.cc/en/Refere...

 

So if you want to work on a 0..399 scale you can. I imagine:

analogWrite(PWM,       map(100, 0, 399, 0, 255));  //25% Modulation depth
analogWrite(DCM_PWM,   map(200, 0, 399, 0, 255));  //50% Modulation depth
analogWrite(PWM_SPARE, map(300, 0, 399, 0, 255));  //75% Modulation Depth

should deliver the expectation shown in the comments.

BTW - sweet irony - but look at the example they give for the map function:

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
The documentation for analogWrite() is unequivocal:

As I have already pointed out ;-)

 

clawson wrote:
100/255 will give 39% duty not 25%

No, as I understand it he has "overridden" Arduinos setting of TOP to 255 with his own 400.

 

I take it he actually wants a resolution of 400 steps. That is quite possible to do with Timer1, with simple code. He is already more than half-way there by overriding Arduinos TOP value. He only needs to realize that with that overridong of the TOP value he has also pulled the rug under AnalogWrite and needs to replace that with his own code. It will be less than 20 lines. I have sketched it above.

 

But... He seems hell-bent on using half of Arduino half of his own code without wanting to understand what Arduinos alanogWrite() assumes or the consequences of such an action.

"He used to carry his guitar in a gunny sack, or sit beneath the tree by the railroad track. Oh the engineers would see him sitting in the shade, Strumming with the rhythm that the drivers made. People passing by, they would stop and say, "Oh, my, what that little country boy could play!" [Chuck Berry]

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Guess I should have read all the thread. (or rather the hijack)

 

So a quick glance through and I even wonder why bother to try and re-use analogWrite() anyway? Writing your own setDuty() is about 3 lines of C isn't it?