dbuezas / arduino-web-timers

Application to configure and visualise timers. Support for Atmel atmega328p (e.g Arduino Uno, Nano, Pro-mini) and Logic Green lgt8f328p
46 stars 10 forks source link

Output port switching code seems wrong in some cases. #22

Closed LaZsolt closed 1 year ago

LaZsolt commented 1 year ago

After you (@dbuezas) mentioned your Arduino Web Timers App, I tested it with Firefox. The result is here:

Timer 1: kép I'm not tested, but in my opinion the correct program code would be:

  PMX0 = 
    1 << WCE;
  PMX0 = 
    1 << C1BF4;

Like setting of the clock prescaler register WCE meaning: update enable then update. PMX0/1update: Must write logic one to WCE. In the following 6 system clocks PMX0/1 update will be finished by writing the register.


Timer 2: kép In my opinion the correct program code would be:

  PMX0 = 
    1 << WCE;
  PMX1 = 
    1 << C2BF7;

Timer 0, no port switching code: kép


Timer 3 looks good code: kép

dbuezas commented 1 year ago

Oh I see, thanks, I'll implement a fix soon.

dbuezas commented 1 year ago

Hey @LaZsolt, do you have an oscilloscope at hand and could confirm your suggestions? I don't have my LGT boards at the moment and can't test this. If you do, I'll highly appreciate the help, then I'll make a release with the fix.

Thank you for the issue anyway in case you can't :)

LaZsolt commented 1 year ago

Hi!

I do not have an oscilloscope. I will try it with a transistor-amplified speaker. I'll set the PWM frequency to a sound frequency.

dbuezas commented 1 year ago

Thanks! I just remembered I made this lgt oscilloscope app: https://github.com/dbuezas/arduino-web-oscilloscope

dbuezas commented 1 year ago

I took a look at the datasheets and it is clear you are correct, so I will merge this fix. There is one thing I am not sure about: Pin E4 in Timer0. This one doesn't require touching the PMX0 register, so I think it shouldn't require preparation with 1 on WCE. Do you know if this is correct?

dbuezas commented 1 year ago

This one I mean: https://dbuezas.github.io/arduino-web-timers/#mcu=LGT8F328P&timer=0&OCnA_OutputPort=E4

image image
LaZsolt commented 1 year ago

There is one thing I am not sure about: Pin E4 in Timer0. This one doesn't require touching the PMX0 register, so I think it shouldn't require preparation with 1 on WCE. Do you know if this is correct?

Yes it will work after reset...

But a minute later it occured to me to ensure the right settings the zero value bit must be written too not only the one value bit.

dbuezas commented 1 year ago

Not sure I'm following

LaZsolt commented 1 year ago

Timer 0 compare A output pin mux test works fine:

// Timer 0 output mux test

#include "wiring_private.h"

void setup() {
  Serial.begin(38400);
  pinMode(LED_BUILTIN,OUTPUT);
  digitalWrite(LED_BUILTIN,HIGH);
  delay(10000);             // Wait 10 seconds after reset for reprogramming

  noInterrupts();
  TCCR0A = 1 << COM0A0;
  DDRC =   1 << DDC0;
  DDRD   = 1 << DDD6;
  DDRE   = 1 << DDE4;
  OCR0A  = 125;
  interrupts();

} // end setup()

void loop() {
  unsigned char tmp;

  // 1. alternative output E4
  noInterrupts();
  TCCR0B = 1 << OC0AS | 1 << CS02 | 0 << CS01 | 0 << CS00;
  tmp  = PMX0 & ~(1 << C0AC0); // Save SSOP20 alternative outputs for SS, TXD, RXD
  PMX0 = 1 << WCE;
  PMX0 = tmp;
  interrupts();

  digitalWrite(LED_BUILTIN,LOW);
  waitasecond();

  // 2. alternative output C0
  noInterrupts();
  TCCR0B = 0 << OC0AS | 1 << CS02 | 0 << CS01 | 0 << CS00;
  tmp  = PMX0 | (1 << C0AC0); // Save SSOP20 alternative outputs for SS, TXD, RXD
  PMX0 = 1 << WCE;
  PMX0 = tmp;
  interrupts();

  digitalWrite(LED_BUILTIN,HIGH);
  waitasecond();

} // end loop()

void waitasecond(void) {
  uint16_t n=1000;
  while (n-->0) delayMicroseconds(1000);
}
LaZsolt commented 1 year ago

Hi David!

Today I finished testing the outputs change functionality for all timer compare outputs. I learned a lot about how counters work. Without your web timers webpage it is 1000 times more difficult to use these timer functions.

When I tested timer 3 A output, I noticed some minor bug. (The timer 3 output A tester program is at the end of this comment.) On the picture below, you can see the A ouput pin number in the red circle is incorrect. In the LQFP48 package the correct pin number is 15. This pin the output of Analog Comparator and alternatively Timer 3 Compare Output (ACOP/OC3A). In the LQFP32 package (and in SSOP20) this pin and port D6 output connected together to pin 10 (SSOP20 pin 9).

So it must not be set DDD6 bit. Tested. (Marked with green frame.) Click to enlarge.

kép

// Timer 3 output mux test

void setup() {
  Serial.begin(38400);
  pinMode(LED_BUILTIN,OUTPUT);
  digitalWrite(LED_BUILTIN,HIGH);
  Serial.println(F("Timer 3 output F1(D1) - (D6) OC3A test."));
  Serial.println(F("Waiting..."));
  delay(10000);             // Wait 10 seconds after reset for reprogramming

  noInterrupts();
  TCCR3A = (1 << COM2A0);
  TCCR3B = (0 << CS32) | (0 << CS31) | (1 << CS30);
  OCR3A  = 27900;
  interrupts();

  Serial.println(F("Running..."));
} // end setup()

void loop() {
  unsigned char tmp0,tmp1;

  // Default output F1 ( = D1 = RXD ) LQFP32
  noInterrupts();
  tmp0  = PMX0 |  (1 << TXD6); // Save SSOP20 alternative outputs for SS, RXD --- TXD pin redirect to D6
  tmp1  = PMX1 & ~(1 << C3AC);
  PMX0 = 1 << WCE;
  PMX0 = tmp0;
  PMX0 = 1 << WCE;
  PMX1 = tmp1;
  DDRF   = (1 << DDF1);
  interrupts();

  digitalWrite(LED_BUILTIN,LOW);
  waitasecond();

  // Alternative output ( D6 ) = ACOP/OC3A LQFP32
  noInterrupts();
  DDRF = 0;
  tmp0 = PMX0 & ~(1 << TXD6); // Save SSOP20 alternative outputs for SS, RXD --- TXD pin restore to D1
  tmp1 = PMX1 |  (1 << C3AC);
  PMX0 = 1 << WCE;
  PMX0 = tmp0;
  PMX0 = 1 << WCE;
  PMX1 = tmp1;
  interrupts();

  digitalWrite(LED_BUILTIN,HIGH);
  waitasecond();

} // end loop()

void waitasecond(void) {
  uint16_t n=1000;
  while (n-->0) delayMicroseconds(1000);
}
dbuezas commented 1 year ago

Wow! Awesome, thanks for the comprehensive testing!

It is indeed super hard to learn all the lgt timer details (and even those of the way simpler atmega to be honest), that's why I made this. And yes, I learned a lot myself by programming this, but even more by using it!

I don't know if you saw it, but the way it works is not by me programming each possible configuration. Instead of that, I grabbed all tables from the datasheets and put all the info in these tables (there's one per timer, per microcontroller). I added extra columns for actual behaviour too, and then a constraint solver finds which behaviour is determined by all the registers.

All of this means that I didn't quite knew what all meant until I could click around myself. I'm super happy about this project :)

Thanks for sharing your thoughts! :)

dbuezas commented 1 year ago
image

Look, I already had an entry for that output and I probably didn't even realise I wasn't using it. It's also written wrongly (a zero instead of the letter o).

I assume it needs the WDR bit too as it is inside PMX1 (and you have it in your code)

dbuezas commented 1 year ago

This should be fixed now:


image image
image image
LaZsolt commented 1 year ago

Look, I already had an entry for that output and I probably didn't even realise I wasn't using it. It's also written wrongly (a zero instead of the letter o).

Sorry to bother you again, but I noticed that that letter o is acually zero. I found in the translated english databook every ACOP, in the chinese text is AC0P. Analog Comparatorzero Positive input. (The other comparator input is AC1N. Analog Comparatorone Negtive input.)

dbuezas commented 1 year ago

Ohhh, that was the timer number! I'll fix it, thanks for letting me know.

You are sorry? No need I'm thankful for your feedback and insights

dbuezas commented 1 year ago

This should be fixed now. Deploy should take a minute. Thanks again for getting back with this 👍