arduino / ArduinoCore-megaavr

Arduino Core for the ATMEGA4809 CPU
103 stars 62 forks source link

Arduino Wifi Rev2, bug when reading duty cycle value on pin 6 #91

Open landret opened 3 years ago

landret commented 3 years ago

Hi, I am trying to read the duty cycle set for pin 6 on the Arduino Wifi Rev2. By default, the timer used is TCB0, used in 8-bit PWM mode. The 16-bit Compare channel CCMP is used as two 8-bit registers, the lowest byte CCMPL is for the period, while the highest byte CCMPH is for the duty cycle.

However, something weird happens when I try to read the Compare channel register, the value of CCMPH is unexpecteadly copied into CCMPL. The same happens with pin 3 (which uses TCB1).

// PWM on pin 6, timer TCB0
//TCB_t* timer_B0 = (TCB_t *)&TCB0; // pointer on timer structure

void setup() {
  pinMode(6, OUTPUT);
  Serial.begin(9600);
}

void loop() {
  setPWMValueOnPin6(20);
  setPWMValueOnPin6(40);
  setPWMValueOnPin6(60);
  setPWMValueOnPin6(80);
}

void printCompareChannelValues()
{
  Serial.print("Compare channel = ");
  unsigned int r = TCB0_CCMP;
  //byte r = timer_B0->CCMP;
  Serial.println(r);

  Serial.print("Compare channel L = ");
  byte l = TCB0_CCMPL;
  //byte l = timer_B0->CCMPL;
  Serial.println(l);

  Serial.print("Compare channel H = ");
  byte h = TCB0_CCMPH;
  //byte h = timer_B0->CCMPH;
  Serial.println(h);
}

void setPWMValueOnPin6(byte value)
{
  analogWrite(6, value);
  printCompareChannelValues();
  delay(2000);
}

Here is the serial monitor output:

Compare channel = 5375 Compare channel L = 255 Compare channel H = 20 Compare channel = 10260 Compare channel L = 20 Compare channel H = 40 Compare channel = 15400 Compare channel L = 40 Compare channel H = 60 Compare channel = 20540 Compare channel L = 60 Compare channel H = 80 Compare channel = 5200 Compare channel L = 80 Compare channel H = 20 Compare channel = 10260 Compare channel L = 20 Compare channel H = 40 Compare channel = 15400 Compare channel L = 40 Compare channel H = 60 Compare channel = 20540 Compare channel L = 60 Compare channel H = 80 Compare channel = 5200 Compare channel L = 80 Compare channel H = 20

This occurs even when removing the serial functions (I can see that by watching a led connected on pin 6). Is that a normal behavior?

landret commented 3 years ago

The origin of the problem has been found and lies in the analogWrite() function:

/* set duty cycle */
timer_B->CCMPH = val;

Please see details and solutions proposed in this discussion on Arduino Stack Exchange.

To summarize the discussion and according to "Accessing 16-Bit Registers" of the ATMega4809 datasheet

In our case, the write sequence is incomplete and here is what happens:

It results that the high byte value is copied into the low byte value...

To circumvent this, a patch to the analogWrite() function is proposed in the discussion on Stack Exchange. I updated the title of this post as I think it is a bug which should be corrected.

facchinm commented 3 years ago

@landret thanks for the extensive clarification. Would you or @timemage for Stack Exchange post a PR for proper attribution? Thanks!

SpenceKonde commented 3 years ago

It is worth noting that this behavior when a TCB is configured for 8-bit PWM - despite the fact that it is hardly news - has recently begun showing up on the Silicon Errata sheets. for 4809: https://ww1.microchip.com/downloads/en/DeviceDoc/ATmega4808-09-SilConErrataClarif-DS80000867B.pdf They say "The errata described in this document will likely be addressed in future revisions of the ATmega4808/4809 devices."

They give no indication of whether this will occur in our lifetime - I can't say the history of the modern AVR or "megaavr" architecture leaves me terribly optimistic.