Quote from arcachofo on September 2, 2024, 7:04 pm

Hi.

I'm having a look at this issue and definitely there is something going on, but not sure if it is related to updating OCR0A and missing compare match.
For example this code seems to work ok:

#include <avr/io.h>
#include <avr/interrupt.h>

ISR(TIMER0_COMPA_vect) {
  PINB = (1 << PINB4); //flip
  OCR0A--;
}

void setup() {
  OCR0A = 254; // arbitrary, before 0xFF
  TCNT0 = 0;
  TCCR0B = 5; // prescaler 
  TCCR0A = 2; // CTC mode
    
  TIMSK = (1 << OCIE0A); //enable interrupt
  
  DDRB = (1 << PINB4); //pin output
  PINB = (1 << PINB4); // flip (on)  
  sei();
}

void loop(){}

int main() {
  setup();
  for(;;) loop();
}

 

#include <avr/io.h>
#include <avr/interrupt.h>

ISR(TIMER0_COMPA_vect) {
  PINB = (1 << PINB4); //flip
  OCR0A--;
}

void setup() {
  OCR0A = 254; // arbitrary, before 0xFF
  TCNT0 = 0;
  TCCR0B = 5; // prescaler 
  TCCR0A = 2; // CTC mode
    
  TIMSK = (1 << OCIE0A); //enable interrupt
  
  DDRB = (1 << PINB4); //pin output
  PINB = (1 << PINB4); // flip (on)  
  sei();
}

void loop(){}

int main() {
  setup();
  for(;;) loop();
}

Quote from Coethium on September 3, 2024, 4:10 pm

Hi,

your sketch helped me to investigate near raises functions. Here what i observed on my sketch (without TCNT0=130, less code helps to investigate) :

==> when i speak of OcUnit, i only mean OCA

When TCNT0==128

• - OCR0A change to 254
• - this calls AvrTimer8bit::topReg0Changed with 254
  • - this function calls McuTimer::sheduleEvents()
    • - this function uses m_counterVal to compare (~TCNT0) ; but this value is 127 !
    • - it reshedule (remove old and add) a raise in 254-127 = 127 timer ticks
    • - then it calls McuOcUnit::sheduleEvents() with 127 as counter value
      • - this function reshedules (remove old and add) raise in 254-127 time ticks (+ 1 cpu tick)
  • - then we update RAM register with McuOcUnit::ocrWriteL() with 254
    • - this call McuTimer::getCount() to know the counter
      • - this call McuTimer::updtCount()
        • - this call McuTimer::calcCounter()
          • - which calculate m_counterVal based on a rescaled difference of simulation ticks (ps) : m_ovfCycle-Simulator::self()->circTime()
          • - m_counterVal = 126 ! (254-128), 128 = rescaled difference of simulation ticks
      • - 126 is used as a counter value for McuOcUnit::sheduleEvents()
        • - so next OcUnit event will come in 254-126 = 128 ticks

So the new Timer event/raise occurs before the new OcUnit event (127 < 128). As the Timer event/raise resets the schedule of OcUnit ; the OcUnit event/raise never occurs.

Now with your sketch :

At first interrupt

• - OCR0A change to 253
• - this calls AvrTimer8bit::topReg0Changed with 253
  • - this function calls McuTimer::sheduleEvents()
    • - this function uses m_counterVal to compare (~TCNT0) ; this value is 0 !
    • - it reshedule (remove old and add) a raise in 253-0 = 253 timer ticks
    • - then it calls McuOcUnit::sheduleEvents() with 0 as counter value (we can ignore this call, i will be reset a few steps below)
      • - this function reshedules (remove old and add) raise in 254-0 time ticks (+ 1 cpu tick) (here 254 is the old value of the match)
  • - then we update RAM register with McuOcUnit::ocrWriteL() with 253
    • - this call McuTimer::getCount() to know the counter
      • - this call McuTimer::updtCount()
        • - this call McuTimer::calcCounter()
          • - which calculate m_counterVal based on a rescaled difference of simulation ticks (ps) : m_ovfCycle-Simulator::self()->circTime()
          • - but as m_ovfMatch (254) is not > cycles2Ovf (253) ; m_counterVal keeps the value 0
      • - 0 is used as a counter value for McuOcUnit::sheduleEvents()
        • - so next OcUnit event will come in 253-0 = 253 ticks

So Timer and OcUnit events/rises occurs at the same time. So which is the first one really served ? If this this the Timer one, it will reset the OcUnit one. Fortunatly, when two events are added to the same simulation tick the second added event is inserted before the first (in Simulator::addEvent()).

Some questions / points of interest :

• - is it wanted the m_counterVal drifts away from TCNT0 ?
• - it's difficult to know which is the "real" value of the internal counter (stored, cached, calculated)
• - Overflow (just after =) isn't the same as Compare Match (when =) ; it seems the two are mixed (not sure)...
• - ... maybe because the Timer class drives some concepts of OcUnitA ?
• - Of course, each mode (Normal, FastPWM, PhaseCorrect, CTC, ...) has its specifics behaviors ; so the difficulty grows.

Hope this can help

Quote from arcachofo on September 3, 2024, 4:12 pm

I just commited the solution for this issue.
Let me have a look to your post...

Quote from Coethium on September 3, 2024, 4:24 pm

Are you interested by the same analysis with your solution ?

Quote from arcachofo on September 3, 2024, 4:34 pm

Timers are a little bit tricky...
Let me follow your analysis step by step.

In the meanwhile some comments on your post:

First, the error was caused by McuOcUnit::m_comMatch updated after McuTimer::sheduleEvents().
But this is a problem only in some specific cases, for example this works ok:

  if (TCNT0==128) {
    OCR0A--;   // force an update into simulide (new value)
    OCR0A=250; // restore value
    TCNT0=130; // force skip on next loop (another bug with 129 or without forcing this skip (= keeping 128))
  }

is it wanted the m_counterVal drifts away from TCNT0 ?

Noy sure what do do you mean by "drift".
m_countVal is not an specific register, for 16 bit Timers it is the combination of 2 registers.

it's difficult to know which is the "real" value of the internal counter (stored, cached, calculated)

Yes it is indeed.
A real pain in the but.

Overflow (just after =) isn't the same as Compare Match (when =) ; it seems the two are mixed (not sure)...

Overflow is managed in McuTimer::runEvent() and Compare Match in McuOcUnit::runEvent().
Not sure what yoiu mean by "mixed".

... maybe because the Timer class drives some concepts of OcUnitA ?

Can you explain what you mean by this?

 

 

  if (TCNT0==128) {
    OCR0A--;   // force an update into simulide (new value)
    OCR0A=250; // restore value
    TCNT0=130; // force skip on next loop (another bug with 129 or without forcing this skip (= keeping 128))
  }

Quote from Coethium on September 3, 2024, 5:04 pm

Thank you for your post. I will make some new tests, and will try with your new commit.

 

Noy sure what do do you mean by "drift".
m_countVal is not an specific register, for 16 bit Timers it is the combination of 2 registers.

For exemple, when TCNT0==128, m_countVal can hold 126 (maybe this is corrected with your commit, i'll look at it)

Overflow is managed in McuTimer::runEvent() and Compare Match in McuOcUnit::runEvent().
Not sure what yoiu mean by "mixed".

Can you explain what you mean by this?

I mean that McuOcUnit::sheduleEvents() sometimes (not always) depends of the behavior of McuTimer::sheduleEvents().

And also this comment in McuCreator.ccp :

787| if( topReg0=="" && ocName.endsWith("A") ) // OCRA managed in Timer

 

Quote from arcachofo on September 3, 2024, 5:32 pm

• - OCR0A change to 253
• - this calls AvrTimer8bit::topReg0Changed with 253
  • - this function calls McuTimer::sheduleEvents()
    • - this function uses m_counterVal to compare (~TCNT0) ; this value is 0 !

Didn't catch this one, but calling m_OCA->ocrWriteL( val ) before McuTimer::sheduleEvents() updates the counter value and should fix that too.

Timers are very tricky because they only update the counter only when it is needed.
In this case I didn't update it before calling sheduleEvents().

About your last post:

For exemple, when TCNT0==128, m_countVal can hold 126 (maybe this is corrected with your commit, i'll look at it)

Ok, some clarifications:
- The counter value is held at m_countVal.
Is this the "real" value?
It does not increases with MCU clocks, it is only calculated when it is needed.
So when it is not needed it will hold the last value used, not the "real" one.
But when it is needed it should hold the "real" value.

- TCNTx registers are updated only if someone reads or write those RAM addresses.
So it can happen that values are different as soon as you don't read TCNTx, but if you read then they should be updated, so you should never know, if that makes sense...

This approach is tricky: very easy to miss something, do some misscalculation, etc.
But updating the counter value and registers to be the always the "real" ones (at every clock cycle) takes a massive amount of cpu and can make the simulation much slower.

 

I mean that McuOcUnit::sheduleEvents() sometimes (not always) depends of the behavior of McuTimer::sheduleEvents().

Yes, if the Timer has to re-schedule because something changed, then OCunits will need to adjust to the changes.
But each one takes care of their stuff.

And also this comment in McuCreator.ccp :

787| if( topReg0=="" && ocName.endsWith("A") ) // OCRA managed in Timer

Yes OCRA is used for the Timer Overflow and for the OCunits Compare Match.
So in this case the Timer do it's stuff and tells the OCunit that OCRA has changed.

 

 

Quote from Coethium on September 4, 2024, 8:49 pm

Thanks for the clarification !

I did some new tests, i gone deeper into the code. As you said this is very tricky (but clever !). The update of yesterday works way better, but still has a little bug.

For this sketch :

[... ISR, setup(), main() as in the previous attachment ...]

void loop() {
  if ((TCNT0==10)&&(!modified)) {
    OCR0A--;   // force an update into simulide (new value)
    modified = true;
  }
}

Here is the debug log of usage of calcCounter() (big values have a comma for readability) :

	circTime	m_OvfMatch		m_ovfCycle		cycles2Ovf (float)	cycles2Ovf (int)	m_Counter
	1,057	253		260,128		252,9990234375	252	1
	2,081	253		260,128		251,9990234375	251	2
	3,105	253		260,128		250,9990234375	250	3
	4,129	253		260,128		249,9990234375	249	4
	5,153	253		260,128		248,9990234375	248	5
	6,177	253		260,128		247,9990234375	247	6
	7,201	253		260,128		246,9990234375	246	7
	8,225	253		260,128		245,9990234375	245	8
	9,249	253		260,128		244,9990234375	244	9
	10,273	253		260,128		243,9990234375	243	10
OCR0A--
	10,282	252		260,128		243,9902343750	243	9
	11,307	252		260,138		242,9990234375	242	10
	12,331	252		260,138		241,9990234375	241	11
	13,355	252		260,138		240,9990234375	240	12

Just after the instruction OCR0A-- ; m_Counter decrements (and this value is visible into TCNT0 in the Mcu Monitor). Why ? The calculation of m_Counter is based on m_ovfCycle ; but at the time of the calculation this variable hold ~260,xxx. But as we update OCR0A, the new ovf shoud be about 1 cycle before (~259,xxx). When sheduleEvent came in action, it does an "opposite" calculation to recalculate m_ovfCycle from the bad m_Counter value, so this value does not change (well it changes just a little, see later). Of course, after the first overflow/compare match ; the new cycle will be good.

Is it a big deal ? One tick skipped in one cycle ? At first no, but let's try another sketch :

void loop() {
  if ((TCNT0==10)) {  
    OCR0A--;   //force update in simulide (as same value skips code for optimisation)
    OCR0A=253; 
  }
}

This sketch isn't really good : the Timer is prescaled, this code change OCR0A many times when TCNT0==10. But it works on a real Mcu (the debug LED in PB4 blinks).

Here the debug log :

	circTime	m_OvfMatch		m_ovfCycle		cycles2Ovf (float)	cycles2Ovf (int)	m_Counter
	8,225	253		260,128		245,9990234375	245	8
	9,249	253		260,128		244,9990234375	244	9
	10,273	253		260,128		243,9990234375	243	10
OCR0A--
	10,278	252		260,128		243,9941406250	243	9
OCRA=253
	10,279	253		260,134		243,9990234375	243	10
OCR0A--
	10,287	252		260,135		243,9921875000	243	9
OCRA=253
	10,288	253		260,143		243,9990234375	243	10
OCR0A--
	10,296	252		260,144		243,9921875000	243	9
OCRA=253
	10,297	253		260,152		243,9990234375	243	10
OCR0A--
	10,305	252		260,153		243,9921875000	243	9
OCRA=253
	10,306	253		260,161		243,9990234375	243	10
a few time later…
OCR0A--
	13,221	252		263,069		243,9921875000	243	9
OCRA=253
	13,222	253		263,077		243,9990234375	243	10

After a few time, circTime passes the "TCNT0==10" test, but m_Counter is always calculated as ~9 ~10. We can see m_ovfCycle slowly grows, as a result of a "ping/pong" calculation between calcCounter() and sheduleEvents(), so the difference remains the same. The simulated Mcu Timer is stuck in this state.

 

[... ISR, setup(), main() as in the previous attachment ...]

void loop() {
  if ((TCNT0==10)&&(!modified)) {
    OCR0A--;   // force an update into simulide (new value)
    modified = true;
  }
}

void loop() {
  if ((TCNT0==10)) {  
    OCR0A--;   //force update in simulide (as same value skips code for optimisation)
    OCR0A=253; 
  }
}

Quote from arcachofo on September 4, 2024, 9:16 pm

Wow, thank you very much for that deep investigation.

I still don't understand why: "But as we update OCR0A, the new ovf shoud be about 1 cycle before (~259,xxx)."
I have to go through it to understand exactly what is happening, but definetly there is a problem there.
Even if it is only 1 cycle it is still a bug, and as you demonstrated, in some cases it can lead to huge differences with the real device.

Also often there are redundant recalculations depending on the source of the changes.
This is not only inefficient but makes debugging even more painful.
I will try to optimize it a bit...