SpenceKonde / megaTinyCore

Arduino core for the tinyAVR 0/1/2-series - Ones's digit 2,4,5,7 (pincount, 8,14,20,24), tens digit 0, 1, or 2 (featureset), preceded by flash in kb. Library maintainers: porting help available!
Other
544 stars 141 forks source link

frequancy measurement for Attiny3226 #990

Closed elektroboard closed 11 months ago

elektroboard commented 11 months ago

Hello.

I want to measure the frequency of a signal applied to port PA5 (or any other port) with Attiny3226. However, I was not successful. There is an example below but it doesn't work, I think I'm doing something wrong. Can you help me?

#include <Event.h>
int width;

ISR(TCB0_INT_vect){
 // TCB0.EVCTRL ^= TCB_EDGE_bm; // we want to listen for both edges, flip trigger
    width = TCB0.CNT;
    TCB0.INTFLAGS = TCB_CAPT_bm;
}

void setup() {
  Serial.begin(9600);
  Serial.println("RESTART");

  pinMode(PIN_PA5,INPUT);

  TCB0.INTCTRL =                  TCB_CAPT_bm; //
  TCB0.EVCTRL  =                TCB_CAPTEI_bm; //
  TCB0.CTRLB    =   TCB_CNTMODE_FRQ_gc; //
  //                                                    ^^ wrong mode. you want input capture on event (
  TCB0.CTRLA    =   TCB_CLKSEL_CLKTCA_gc | TCB_ENABLE_bm;   // use clock of TCA0
  //                                 ^^ Why??? 
  TCB0.EVCTRL  |= (    1 << TCB_CAPTEI_bp | 1 << TCB_EDGE_bp  | 1 << TCB_FILTER_bp);
 //Negative Edge : Initialize,Positive Edge : Capture = count, interrupt Input Capture Noise Cancellation Filter: enabled */
  TCB0.CTRLA |= (TCB_CLKSEL_CLKDIV1_gc  | 1 << TCB_ENABLE_bp | 1 << TCB_RUNSTDBY_bp | 0 << TCB_SYNCUPD_bp); 
    /* CLK_PER/2, Enable: enabled, Run Standby: enabled, Synchronize Update: disabled */
   //   ^^ CLK_PER/2 says comment, CLK_PER/1 says code! -Spence
  sei();
//^^ No effect, there's no cli!
}

void loop() {
  Serial.print("frequency = ");
  Serial.print(4*78125/width); // 20 MHZ AND CLOCK/256
  Serial.print(" Hz, ");
  Serial.print(4687500/width);
  Serial.println(" rpm");
  delay(1000);
}

Edited 1. to add quote tags, 2. to mark some issues inline and 3. to delete a bunch of lines that look like code but are commented out -Spence

hmeijdam commented 11 months ago

I would start with changing:

int width;

to

volatile int width;

https://www.arduino.cc/reference/en/language/variables/variable-scope-qualifiers/volatile/

SpenceKonde commented 11 months ago

The big problems I see are:

  1. You are using frequency measurement mode, but you appear to be trying to get pulse widths. I think your commented out plan with EDGE getting flipped back and forth is probably a better approach. You need to store more than that though - you've gotta track the value that was in CNT cause it doesn't reset in this mode subtract previous cnt from current cnt as uint16_t, and rollover or not, as long as the event is less than 2^16 ticks long you're fine. You can double that maximum on the 2-series by then casting to a uint32_t, then checking and clearing the TCB OVF flag, then adding 2^16 that result if OVF wasn't set.

  2. You didn't set up the EVSYS, every line that might have done that was 1. commented out, and 2. for the 1-series

    EVSYS.CHANNEL0 = EVSYS_CHANNEL0_PORTA_PIN5_gc;
    EVSYS.USERTCB0CAPT = EVSYS_USER_CHANNEL0_gc;

    Actually, the new style names work on 0/1-series too on mTC. CHANNEL0 and 1 are sync channels, channel 2-5 are ASYNC, and you still do have to keep that in mind, but porting code is a lot less painful when the names have the same format, and probably the same name (though the selection of generators available to each channel is not the same alas).

  3. variables modified in an ISR and read outside of it must be declared volatile. Variables read within an ISR, and written from outside the ISR, if larger than 1 byte, must disable interrupts while writing them outside the ISR, unless the larger-than-1-byte thing is an SFR (special function register), where the hardware ensures this (see "Accessing 16-bit variables" in the datasheet - though note that writing normal C code, you can write a 16-bit value to a 16-bit SFR without worrying about it writing the bytes in the wrong order, it doesn't do that. The hardware also synchronizes them so they appear in the actual register on the same clock cycle. (sidenotes a and b are related, but are not at issue here, just somethings you should be aware of. a. Note that in the TCA and TCB, depending on mode, conversion of certain registers between a pair of 8 bit registers and a single 16-bit one can occur - or should. It works on the TCA's 5 (?) registers when in split mode, they become 8-bit, but the TCB's CCMP register is supposed to be split into 8-bit registers by 8-bit PWM mode. That doesn't happen - it remains 16-bit in PWM mode, and so you need to write CCMPL followed by CCMPH any time you want to change either value, the change takes effect when CCMPH is written. Or use a union to b. 16-bit SFR writes usually do not need to be guarded by disabling interrupts because of the "writing to 16-bit registers" mechanism, but it does still need to be guarded with disabling interrupts in the case that an interrupt in the program ALSO writes to any 16-bit or larger register on the same instance of the same peripheral.if there is interrupt code that reads or writes to registers larger than 8 bits on the same instance of the same peripheral - this is because a single temp register gets shared by a given instance of a given peripheral.

  4. You need to turn off interrupts when configuring the interrupts on timers. You seem to know that, since you put an sei at the end.... but where is it's cli()? Interrupts are enabled when setup starts (I think it's the last thing that happens before we call setup() - (otherwise until you'd done sei(), millis wouldn't tick up)

  5. There is no rhyme nor reason to your organization of writes to the peripheral registers. But this is a place where that shouldn't be the case - you need to do that with some care to ensure that nothing is changed when it could cause a bunk interrupt: First disable the timer(s) you're working on. then set up everything except for CTRLA, and the INTCTRL and INTFLAGS. Then disable interrupts, set intctrl, set CTRLA (to the value you calculated already), and TCxn.INTFLAGS = TCxx.INTFLAGS (to clear all flags that may have gotten spuriously set. I think you only need 6 lines guarded with the cli/sei.

  6. This is the year 2023, and this is a modern AVR. Not the problem at hand, but.... you don't still need to default 9600 baud. You are more likely to cause problems than prevent them by using 9600 baud (due to the buffer filling and then the sketch execution speed being limited to the speed of it'sserial output). (I've been trying to set all my examples to 115200 baud which works at all supported speeds, even 1 MHz on a modern AVR.

Modern and classic have the same maximum baud rate from a cursory glance at features - F_CPU/8 - but only modern AVRs can actually use normal baud rates anywhere near there. Classic AVR USART needed to know what integer to divide it's clock by to get a clock running at 1/8th or 1/16th the UART baud rate Well, when you're close to the maximum, you can use either the maximum baud rate or half of that, but nothing in the middle. Modern AVR s instead need the number of 64ths of clock cycles to divide the clock by, which means that even the pessimal case, ideal value 64.5, baud rate error is limited to under 1%, and in realistic cases (lower baud to clock ratio, and, with accuracy improving as baud rate decreases (as you would expect) such that the baud calculation error is negligible. The USART was one of the most-improved peripherals on the first modern AVRs.

elektroboard commented 11 months ago

Frankly, I'm a little confused. Could you add an example for what I want to do concretely?

SpenceKonde commented 11 months ago

I don't have one, I never have wanted to measure a frequency - usually I'm running in event capture mode and getting each length.

hmeijdam commented 11 months ago

here is an example