Migrating from Arduino Timer2 to Attiny 1626 Timer1

[Eddiiie]

Hi,
I have a library that uses Timer2 on an uno. An 8 bit timer. The ATtiny 1626 uses 16 bit timers and has a split mode.
By doing some reading Timer1 on the 1626 is used for tone() which I do not use. Are there any settings I need to change under Tools in the Arduino IDE such as mills() and micros() ?

Assuming a 16Mhz crystal, can someone give me an example code showing timer1 that fires its ISR every 4 - 6ms?

The ISR calls a function that reads the value of an analog pin.

Thank you.

Here is the original code -

  // Setup timer2 -- Prescaler/256
  TCCR2A &= ~((1<<WGM21) | (1<<WGM20));
  TCCR2B &= ~(1<<WGM22);
  TCCR2B = (1<<CS22)|(1<<CS21);      

  ASSR |=(0<<AS2);

  // Use normal mode  
  TCCR2A =0;    
  //Timer2 Overflow Interrupt Enable  
  TIMSK2 |= (0<<OCIE2A);
  TCNT2=0x6;  // counting starts from 6;  
  TIMSK2 = (1<<TOIE2);    

  SREG|=1<<SREG_I;

The ISR

// Timer2 interrupt routine -
// 1/(160000000/256/(256-6)) = 4ms interval

ISR(TIMER2_OVF_vect) {  
  TCNT2  = 6;
  update_adc_key();   // update the value of the reading of the analog pin
}

That is really code to me, no idea.

[SpenceKonde]

Like I asked when you emailed me, please post the contents of update_adc_key() - because I think there might be a much more graceful way to do this with a lot less code.

The type B timers are excellejt for tasks like this - something like this is the general structure of a TCB and it's all covered in the dataheet

typedef struct TCB_struct
{
    register8_t CTRLA;  /* Control A */
    register8_t CTRLB;  /* Control Register B */
    register8_t reserved_1[2];
    register8_t EVCTRL;  /* Event Control */
    register8_t INTCTRL;  /* Interrupt Control */
    register8_t INTFLAGS;  /* Interrupt Flags */
    register8_t STATUS;  /* Status */
    register8_t DBGCTRL;  /* Debug Control */
    register8_t TEMP;  /* Temporary Value */
    _WORDREGISTER(CNT);  /* Count */
    _WORDREGISTER(CCMP);  /* Compare or Capture */
    register8_t reserved_2[2];
} TCB_t;

Off the top of my head, equivalent code for TCB1 would look something like

#define TICKS_PER_MICRO (F_CPU/2000000UL) //2 million instead of 1 million because prescaling by 2)
#define TIMEOUT_IN_MICROS (4000UL) 
#define TIMEOUT_IN_TICKS  (TICKS_PER_MICRO * TIMEOUT_IN_MICROS) 
#if (TIMEOUT_IN_TICKS > 65535)
    #error "That interval is too long - you'll need use the TCA0 prescaler"
#endif

TCB1.CTRLA = 0;
//make sure it has stopped
// will eventually set to but doingthat in one assinment saves a few bytes/.  TCB_CLKSEL_DIV2_gc || TCB_EMABLE_b; 
TCB1.CTRLB =  0;
TCB1.EVCTRL = 0;
TCB1.INTFLAGS = 0x03; // clear int flags
TCB1.CCMP = TIMEOUT_IN_TICKS;
TCB1.CNT = 0;
TCB1.INTCTRL = TCB_CAPT_bm;
/* Only get prescale of 1, 2 , or "whatever the TCA0 is set to"
 * 
 */

TCB1.CTRLA = TCB_CLKSEL_DIV2_gc || TCB_ENABLE_bm; 

ISR(TCB1_INT_vect) {
    TCB1.INTFLAGS= TCB1.INTFLAGS; /* VERY VERY IMPORTANT - in tis mode you MUST manually clear it!*/
    update_adc_key();
}

but again, what does update_adc_key() do? There may be an easier way.....

[SpenceKonde]

And if update_adc_key() contains analogRead() calls, it has always been bad code. analogRead is slower than anything you want to be doing in an ISR on the classic AVRs.... Here, unless using automatic accumulation, the ADC is fast enough that it isn't so bad to do. (these parts can take any power-of-two number of ADC readings and sum them automatically; obviously accu,iulating 1024 ADC readings would take far too long to do in an ISR)

[SpenceKonde]

Transferred to the megaTinyCore discussions section; the modern ATtiny parts are supported by this core, not ATTinyCore.

[Eddiiie]

Oh, my bad I did not realize you were waiting for an answer from me. Maybe this discussion will help others someday.

You are right. The ISR calls a function that calls another function that does the analogRead along with way too much other b.s. to do in an ISR.

Let me try to get it all.. No, I didnt write it. Just dont want to rewrite it. I am starting to think it should be.

The ISR

ISR(TIMER2_OVF_vect) {  
  TCNT2  = 6;
  update_adc_key();
}

update_adc_key() -

void update_adc_key(){
  int adc_key_in;
  char key_in;
  byte i;

  adc_key_in = analogRead(MY_ANALOG_PIN);
  key_in = get_key(adc_key_in);
  for(i=0; i<NUM_KEYS; i++)
  {
    if(key_in==i)  //one key is pressed 
    { 
      if(button_count[i]<DEBOUNCE_MAX)
      {
        button_count[i]++;
        if(button_count[i]>DEBOUNCE_ON)
        {
          if(button_status[i] == 0)
          {
            button_flag[i] = 1;
            button_status[i] = 1; //button debounced to 'pressed' status
          }

        }
      }

    }
    else // no button pressed
    {
      if (button_count[i] >0)
      {  
        button_flag[i] = 0;  
        button_count[i]--;
        if(button_count[i]<DEBOUNCE_OFF){
          button_status[i]=0;   //button debounced to 'released' status
        }
      }
    }

  }
}

get_key() -

// Convert ADC value to key number
char get_key(unsigned int input)
{
  char k;

  for (k = 0; k < NUM_KEYS; k++)
  {
    if (input < adc_key_val[k])
    {
      return k;
    }
  }

  if (k >= NUM_KEYS)
    k = -1;     // No valid key pressed

  return k;
}

[Eddiiie]

Is this a typo?

// will eventually set to but doingthat in one assinment saves a few bytes/. TCB_CLKSEL_DIV2_gc || TCB_EMABLE_b;

[SpenceKonde]

that sentence you asked about was an utter mess, I think I changed my phrasing several times while writing it and never read the whole thing - it easd meant to explsin why I didnt set CTRLA to TCB_CLKSEL_DIV2_gc on that line .

That code makes my skin crawl.....

(Is that all the ADC is used for, reasdign mltiple keuys with one wire, or is their, or does it get pointed at other pins for other purposes...

[SpenceKonde]

BTW, the reason I asked whether that's all the ADC is used for is that therecould be better ways to implement it if so - especia,ly if you'd kind of like the timer for something else.

You put the ADC into "free running" mode. on that one ADC pin. Kick it off once (you might even want to slow down the ADC clock so the readings don't come in so fast, they come in damned fast on 2-series parts because of the improved ADC) and it will continue sampling until you explicitly tell it not to.... I'll bet something like this would perform a lot better than a direct translation WITHOUT USING A SEPARATE TIMER

setup:
analogReadResolution(12); // may not need it
// by default we try to make the ADC fast... but I think you can do it without the timer by pushing the ADC speed to just about the minimum.
// and getting rid of the analogRead() in the ISR, I suspect, will reduce the time that calculation takes =
ADC0.CTRLF|=ADC_FREERUN_bm | ADC_SAMPNUM_ACC4_gc; //freerun mode accumulate 4 samples
analogSampleDuration(255); // this slows it way down...
analogClockSpeed(400); // at 20 MHz this will give you... I think 357 kHz? 
// At 20 MHz this will give one sample per 4 x (255+13)/357 seconds = approx 3ms. 
analogRead(MY_ANALOG_PIN); // kick off the ADC; it will take a reading periodically and fire this interrupt (which doesnt need to call analogRead and so is much faster) 
ADC0.INTFLAGS =  0x3F; //clear all flags so we have no risk of a bogus interrupt when we fire this....
ADC0.INTCTRL = ADC_RESRDY_bm; // enable the interrupt.... 



ISR(ADC0_RESRDY_vect) {
  int adc_key_in;
  char key_in =-1;
  byte i;

  adc_key_in = ADC0.RESULT;
  key_in = get_key(adc_key_in);
  // IMPORTANT: Code below ASSUMES that adc_key_val[i] is always larger than adc_key_val[i+1] - that is, that the keys go in descending order of ADC values.
  // If it's the other way around, change adc_key_val[i] to adc_key_val[NUM_KEYS - 1 -i] and I think the rest would just work
  for(i=0; i<NUM_KEYS; i++)
  {
    if (input < adc_key_val[i])
    { // Start of "we found a value consistent with a key being pressed" logic
      // Okay I couldn't resist combining the two loops. why loop over the pins twice? the tricky part is that you only want THIS block to run for the first key it finds (cause all subsequent keys will look pressed). There is a simple way to acheive this!
      if(button_count[i]<DEBOUNCE_MAX) 
      {
        button_count[i]++;
        if(button_count[i]>DEBOUNCE_ON)
        {
          if(button_status[i] == 0)
          {
            button_flag[i] = 1;
            button_status[i] = 1; //button debounced to 'pressed' status
          }
        }
      }
    } 
    else // button pressed not pressed
    {
      if (button_count[i] >0)
      {  
        button_flag[i] = 0;  
        button_count[i]--;
        if(button_count[i]<DEBOUNCE_OFF){
          button_status[i]=0;   //button debounced to 'released' status
        }
      }
    }
  }
}

Another thing you could do using freerun mode which is FAR ​less extreme would be to simply do

// Multiply ADC0.
ADC0.CTRLF |= ADC_FREERUN_bm;
analogRead((MY_ANALOG_PIN); in setup, 

and use what was discussed in ppreviouscouple of comments, otherwise EXCEPT that you'd replace the analogRead with ADC0.RESULT;
You'd  always have a fresh result waiting for you in there, so instead of wasting on analogRead,  you'd  just read the register  and have your result. 
. 

either way, that ungodly mess you have there should be taken behind the woodshed and reimplemented - you can only have 1 button pressed at a time right? And there are two variables there that are 1's or 0's, one per button. if there are 8 buttons or less, that should be a single byte for each (otherwise an appropriately sized unsigned int). If you could safely set all except the bitfor the button you now know is pressed isn't pressed, then it's just var = 1<<i or something like that.

shrug but I should go do something more useful. like getting 2.4.0 readty for release - if it worked on classic generation, the differences between them are such that the analogous implementation to what you had on classic will worst case be comparable3 and ;likely faster.

[Eddiiie]

Wow Spence that is quite a bit better than the mess that existed before.
I can't understand why they want put the analog read into a timer. They bragged about interrupt driven button select. But why?
That's not even interrupt driven! Right? Screw the timer, just get the analog value when you need to, after displaying the meun and waiting or an input. There are many analog debouce examples out there.. I do have a function that needs to be called frequently to keep the far-end device my code is talking to from locking up. I can call that function in the same wait loop.. I do like having a heartbeat LED that blinks to show that the system is still alive and could repurpose the timer for that. Like, 250ms on, 250ms off.

I am still in the process of setting up breadboard for testing. There are 2 RS485 chips, RJ45 connectors, this Joystick button w/select, and a Nokia SPI LCD display. Wires everywhere. Might just make the PCB.

I love how there is no need for a pullup resistor on the reset line.

Screw having this in an ISR!!

// Multiply ADC0.
ADC0.CTRLF |= ADC_FREERUN_bm;
analogRead((MY_ANALOG_PIN); in setup, 

and use what was discussed in ppreviouscouple of comments, otherwise EXCEPT that you'd replace the analogRead with ADC0.RESULT;
You'd  always have a fresh result waiting for you in there, so instead of wasting on analogRead,  you'd  just read the register  and have your result. 
. 

I should probably use ADC0.RESULT anyway.

Thank you for the input Spence!

[SpenceKonde]

No prob, glad that was useful And as I said, all the rest of the code in that ISR sucks too (as far as I'm concerned, analog read for buttons is fundamentally a lame approach. I bet if you did like a 4 sample accumulate, and compared ADC0.SAMP with ADC0.RESULT, and then took a ton of data like that you might be able to debounce cleanmly tat way. Or if not, keep a short history of readings, or whatever/ But the real core issue is that analog read is the wrong way to do button input (shrug) To be fair I did occasionally see it when I was scrapping consujmer electronics just to see what design decisions they made (very good exercise!) - though I guess with a 1626, you can at most only get 4 more I/O pins by going with a 1627 - above that you're in 4808 or dx28 or dx32 land

[Eddiiie]

I kinda think it is simpler than that, even.

There will be 5 resistors that represent Up, Down, Left, Right, Select. Just need to check for something in those ranges, maybe check 3 times just to be sure. They are pretty well defined (1% tolerance) values. Right? Thinking of using 1k, 2k, 3.3k, 5k, 10k. Breadboarding this tonight.

I read somewhere you said the crystal pins are for only 1 type of crystal, so I am going to bail on using a 20mhz crystal, the internal one probably just fine for 38,400 baud. This frees up 2 pins! Eliminates .swap , getting off topic.

Let's get rid of the timer for reading the analog pin and eliminate that nasty code.

[SpenceKonde]

Oh for sure, yeah - the crystal pins are only for 32.768kHz watch crystal for the RTC. (though they can take an external system clock input on PA3).

You thought you needed crystal for uart? No, no you don;'t. we're living in the future now the oscillators on these are miles better than classic AVRs. I have literally never had any problems with the factory cal, even without using the oscillator error term from the tools submenu. Like, the oscillators are nothing like what you[re used to from the classic AVRs.

(38400? so slow! These parts will do 115200 baud at (16 MHz / 16 =) 1 MHz from internal oscillator. I in the debug tooling for my tuning stuff that's going in to 2.4.0 I have a 20 MHz sketch that I run it at 921600 baud because it's an SPI slave that takes in debugging output and data for curve fitting over SPI from it's source (which can't use USART because it's cycling MCLKCALIBA register through every option, while timing a stream of 1 ms pulses. AVRs run the SPI bus at fairly high speeds (ie, maximum SPI clock divider, if the chip is lucky and got through it's high side cal range, would exceed 300 kHz), and I did not want to have to implement a buffer do avoid losing data, so I just cranked the USART baud way the hell up (just being a little faster wasn't enough, but I forget if I was using the Arduino serial library, that is slow enough that if you were barely going to keep up based on the baud, the delays from the bloat and inefficiency in HardwareSerial will kill it) - and since it only goes one direction, as long as it can get data out much faster than new data will ever come in, we're golden.

[grandaspanna]

Mainly because I zealously wanted better accuracy, one of my 1614 projects uses an oscillator to feed EXTCLK at 10MHz. Easy enough to do, but oscillators are more expensive than the microcontrollers.

[SpenceKonde]

tell me about it. The new version of my tinyAVR breakout boards is gonna support a 3225 external clock... so I placed some orders for some last week... "digikey wants HOW MUCH for those?! No, really how much are they?... Okay, aliexpress it is" unpleasant place to buy components from because they often don't even have the part number, just "5 pcs active crystal 24 MHz" and the specifications tab says 3.3-5.5V (if you're lucky - often there was no voltage spec on the oscillators, and the guys didn't seem to know either the part number nor the operating voltage at one store I asked). But I found one place that told me the voltage and sold me 55 oscillators, 3225 package for $20 shipped, which is pretty damned good all things considered, and from another seller for my DB boards, I snagged 210 pcs 3225 crystals for a under $ 30 shipped. I was NOT going to pay western world prices for that crap, the prices from digikey were just loony. Crystals, switches;/buttons, and anything mechanical like fastners and especially enclosures, are a fraction of the price they are here in in the far east. On the other hand ICs that are in current production in the west are generally no cheaper there than here and the selection sucks, though native chinese IC's (which aren't as fancy, because china's chip foundries are still pretty far behind - but very often they are perfectly effective for their task), as well as clones of partjs that were discontinued by the original manufacturer long ago (the MAX662 eqiivalent in particular is a really nice part - 5v to regulated 12v without an inductor, 30mA max (switched capacitor topology - was originally used for flash memory that needed 12v erase voltage., and is only produced by maxim in the west now, and they charge an arm, and a leg - but someone's making identical parts in china for a fraction of the price and they work fine. They're what I plan to use if I ever do an HV UPDI programmer.

[SpenceKonde]

Hmmmmm if you could get all three pins onto the same port and use them like this you can ditch analog shit alltogether!

select - one diode from btnpin1.
up - one diode from btnpin2.
down - one diode from btnpin3
left - one diode from btnpin1, another from btnpin2
right - one diode from btmpin1, another from btnpin3.
You get two more options too
one diode from btnpin2, another from btnpin3
one diode from each btnpin.

then it's all digital reads.

In fact if you can maneuver them all to be adjacent on the same port, (say porta pins 1-3) you could

uint8_t btnval=VPORTA.IN; //1 clock 1 word in (VPORTs are in the IO space)
btnval >>= 1;             //1 clock 1 word lsr (rightshift)
btnval ^= 0xFF;           //1 clock 1 word compiles to a single com instruction, rather than ldi, eor (inverts it) 
btnval &= 0x07;           //1 clock 1 word andi (and with immediate)

Now you have 3 bit number that is 0 when nothing is pressed, and a different number dpeending on what button was pressed, in 4 clock cycles per read with 3 pins and a dozen el cheapo signal diodes ("small signal" diodes. I have worked with some low-forward-drop optimized diodes for the t43, the part with the integrated boost converter to run from an a single alkaline battery - but I wanted it to be efficient, so I chose the lowest drop diode I could. Well, it never went into the low power mode, because even holding back 3v, the reverse leakage through the diode was enough that it never went into active low current mode even with fresh batteries, meaning it ws leaking over a third of a milliamp in the reverse direction wehen only holding back 3V. And the datasheet explicitly warns about this and I ignored it too! Worked beautifully with proper diodes once I knew what I had to look for :-P But my point in mentioning it is that those diodes, while great at high current wouldn't actually act like diodes in this application hence why I specificaklky mentioned use of "small signal" diodes../

Heck, imagine strategically chosen combinations?

up - one diode from btnpin1. (1)
right - one diode from btnpin2 (2)
down - one diode from btnpin1, another from btnpin3 (5)
left - one diode from btmpin2, another from btnpin3 (6)
select - one diode from btnpin3. (4)

and the two combinations with btnpin1 and 2 are declared invalid

You could do things like

if (btnval && ((btnval & 3) != 3)) { //don't react to invalid buttons (user is pressing multiple buttons, wait for the idiot to stop doing that, like many devices do).
  //- debounce it some how? 
  if (btnval == 0x04) {
    //select 
  } else if (btnval & 0x01) {
    if (btnval & 0x04) {
      goDown();
    } else {
      goUp();
    }
  } else { // since we only enter this area if btnval is not 0, 3, or 7, and we handled 1, 4, and 5, all that's left is 2 and 6, and the difference is just the highest of the three bits so.... 
    if (btnval & 0x04) {
      goLeft();
    } else {
      goRight();
    }
  }
}

[Eddiiie]

I finished the PCB at like 5AM this morning and sent it off to get made. Good to get that done.
My testing shows that the values read by the analog pin are very consistent. What doesn't make sense to me, though, is I thought by adding bigger and bigger resistor values that the analog reading would go down, or at least go down a lot more than they did. They all seemed to be in the same range-ish. Time to work on the code.

[SpenceKonde]

The ADC is baller, way better than a normal 10-bit one on classic AVR, or the 12-bit fake differential one on the Dx-series. If the numbers you get are unexpected, either it is configured wrong or your test setup is wired incorrectly and the actual analog voltages are not what you expect. You can resolve this mystery for certain with a any DMM - just check the voltage between analog pin and ground. Does it match with what your math predicts for that combination of resistors (I'm assuming this is one supply rail to a bunch of NO switches each of which connects to one side of a common resistor, and the other end of that is connected to the opposite supply rail, such that when nothing it pressed, it's always at either ~0 or ~Vcc, depending on which way around you wired it, and when you press a button, it will take on an intermediate voltage based on ratio of those resistors). You should be able to run simple code to print analog values out and measure the same voltage on your scope that is reflected at the pin (Hopefully at this point, it's not a wiring problems that is now reflected in the board you sent out at 5AM. I have seen people \post just about every permutation of that circuit, even though only one is sane. Also make sure I didn't set default resolution or reference voltage wrong - I don't think I do, but it's new (and when using RESULT directly alongside freerun mode, making sure you get the right resolution is your job, not in the core's control anymore. The hardware natively will give you 8 or 12 bits, and megaTinyCore defaults to emulatiing 10 bit ADC of older parts (from a 12 bit reading) until you instruct otherwise, but one would normally put it into 12-bit mode in setup, (see analogResoliution() in readme, and also be on guard for any default parameter like the reference voltage to be wrong on startup too. Any bug like that should be reported as I am not aware of any.