My Digital Tachometer - putting it all together

Go To Last Post
192 posts / 0 new

Pages

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

aeroHAWK wrote:
The ISR would still need to increment tachCount, so if an I/O register was used, a few more cycles could be saved....
I think you made this comment before your new-found understanding of the limitations of arithmetic operations and their need for general purpose registers. The advantage confered by the use of I/O registers over SRAM is in the time it takes to fetch/save a variable, not in the time it takes to perform the arithmetic.
Quote:
If the saved variable was volatile, wouldn't this be less of an issue?
Volatile is a high-level-language concept. In C, C++ and other languages declaring a variable as volatile tells the compiler that it should not perform certain optimisations. Specifically, the compiler is not allowed to 'optimise away' any access to the variable. Everywhere you place a reference to that variable (read or write), the compiler is obliged to explicitly access the variable.

In the context of your assembler ISR (or indeed any assembler), volatile is meaningless. But in the context of the main-line C code that accesses the same shared variable, volatile is important. Without it, the compiler might 'optimise away' accesses to the variable because its execution path analysis tells it the variable doesn't change. This is because the compiler can't know that the ISR can change the variable too. The ISR is considered to be a different 'thread'. Declaring a variable volatile is a way around this.

So, it wouldn't be 'less of an issue', rather volatile is a requirement when sharing a variable between an ISR and main-line code, but only if one or both of those accesses are done in C.

Search the forums for 'volatile', there are several recent threads pertaining to it. And have a look at @clawson's signature.

Quote:
When I calculated the 'worst case', I was surprised to find the degree of the error, so I think it's worth exploring options.
Another strategy that might bear some scrutiny is 'nested' interrupts.

Normally when an interrupt occurs, interrupts are globally disabled while the interrupt is being serviced. The RETI at the end of the ISR re-enables them. This is good and right, but it means that interrupts which occur while the mcu is already servicing another get deferred. This, as mentioned, may be part of your trouble.

In some circumstance the programmer may decide to deliberately re-enable interrupts in an ISR, either immediately as the first instruction of the ISR or anywhere else before the terminating RETI.

This confers the advantage that any pending interrupts are serviced immediately, instead of after the whole of the current ISR is complete and has returned with RETI. In essence you can limit the additional latency suffered by the pending interrupt to about 7 cycles (the 6 cycle latency of the current interrupt, plus 1 cycle for the SEI which enables global interrupts).

The risk is that if your application receives more interrupts than it can process with available resources (CPU cycles, stack space), your application will fail, probably in a spectacular fashion.

Indeed, the notion of nested interrupts is a controversial one, and I'll probably attract some criticism for suggesting it. These concerns are non unwarranted. A thorough analysis of your applications interrupt behaviour, and cpu, stack, and other resource utilisation is important before you can commit to a nested interrupt approach.

In your case, the timer interrupt is the possible candidate for an early re-enabling of interrupts because it is the INT0 interrupt (and your RPM calculations) that can suffer the most from additional latency.

Quote:
I know it can be easy to get caught-up in the esoteric and loose sight of the real task. :wink:
You and me both ;)

JJ

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
You cannot reserve other registers besides this set.
That's a shame (although not a huge one). There are some instructions that can only operate on certain registers. For example, ADIW and SUBI work only with register pairs R24/R25, R26/R27, R28/R29, and R30/R31. While their byte-wide brothers SUBI and SBCI can perform the same operations on all registers, and at the same speed (ADIW and SUBI each take 2 cycles, and perform the same function as a pair of 1-cycle SUBI/SBCI instructions), they take an extra word of flash to do it. Not a big deal, unless you're trying to squeeze your ISR into the interrupt vector table.

Also, you're limitted to 4 registers in total for your register variables. That should be sufficient for your needs.

By the way, if you're going to venture into the deep dark wonderful world of assembler, keep one of these on your belt.

JJ

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

joeymorin wrote:
I think you made this comment before your new-found understanding of the limitations of arithmetic operations and their need for general purpose registers.
Yes, you are correct.
joeymorin wrote:
In the context of your assembler ISR (or indeed any assembler), volatile is meaningless. But in the context of the main-line C code that accesses the same shared variable, volatile is important.
The notion of volatility is new for me, and obviously is still unclear. Thank you for pointing that out. I seem to get it confused with a variable being global. I will need to do more studying. :oops:
joeymorin wrote:
Indeed, the notion of nested interrupts is a controversial one
I thank you for introducing me to this concept, but for now I think it is a little more than I want to chew at this time. :shock: :wink: I suspect streamlining the ISRs will be sufficient to get acceptable performance.
joeymorin wrote:
ADIW and SUBI work only with register pairs R24/R25, R26/R27, R28/R29, and R30/R31.
So the ISR will need to move the bytes before they can be incremented.... Doesn't that eliminate the advantage of reserving the registers for tachCount?
joeymorin wrote:
By the way, if you're going to venture into the deep dark wonderful world of assembler
Thanks JJ, I was really just planning on peering out into the valley... or maybe just a short visit... if that is possible.... :shock:

Cris

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

aeroHAWK wrote:
The notion of volatility is new for me, and obviously is still unclear. Thank you for pointing that out. I seem to get it confused with a variable being global. I will need to do more studying. :oops:
Here's a good place to start.
Quote:
So the ISR will need to move the bytes before they can be incremented.... Doesn't that eliminate the advantage of reserving the registers for tachCount?
This:
SUBI R24, 0x12 ; 1 cycle, 1 word
SBCI R25, 0x00 ; 1 cycle, 1 word

... is equivalent to:

SBIW R24, 0x0012 ; 2 cycles, 1 word

The value 0x0012 is subtracted from the register pair R24/R25. The former takes 2 cycles, and 2 words of flash. The latter also takes 2 cycles, but only takes 1 word of flash, but is restricted to register pairs from R24 onwards, whereas the former can be applied to any register from R16 through R31.

So you won't see a speed penalty due to Imagecrafts R20-R23 limitation.

In any case, the compiler will (probably) choose the optimal instructions based on the registers in question.

JJ

[EDIT: corrections w.r.t limitations of SBCI/SUBI/SBIW]

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

Last Edited: Fri. Nov 15, 2013 - 08:15 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

joeymorin wrote:
... is equivalent to:
SBIW R24, 0x1234 ; 2 cycles, 1 word

No, the immediate constant must be between 0 and 63. If 0x1234 was allowed the instruction wouldn't fit in one word.
Quote:
the former can be applied to any register from R0 through R31.
No, R16 to R31.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If the isr is taking 2-3us, why is this an issue? Especially when measuring around 60Hz?

The signal from the engine will jitter more than that.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

snigelen wrote:
No...
No...
Another hasty reply on my part. Thanks for the correction. I've edited my post for posterity.

JJ

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
If the isr is taking 2-3us, why is this an issue? Especially when measuring around 60Hz?

The signal from the engine will jitter more than that.

joeymorin wrote:
Not that I want to stifle your education on assembler, registers, and ISRs, but you may want to take a step back and analyse were the problem lies, exactly.

If you're seeing problems with accuracy from your rpm calculations, you may consider other approaches beyond hand-optimising ISR code.

Indeed, there are possibly other factors at work.

JJ

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
If the isr is taking 2-3us, why is this an issue?
Due to a temporary condition (a cranial/rectal inversion :shock: ), I mistakenly compared those 2-3 uSec to a single 'tachCount' period (to find the error) instead of comparing it to the 75 mSec counting window. :oops: So rather than the 1.1 % error I thought I had, the actual error is 10 rpm at any speed (one 'tachCount' during the counting window). Besides that, (now that I am relieved of the afore mentioned temporary condition), I see that speeding up the ISR would have no overall effect on that error! :oops: :shock: It would only change the particular counting window it would occur in, not the number of occurrences (plus, the error would be corrected in the very next counting window - it would not accumulate).
Kartman wrote:
Especially when measuring around 60Hz?
Well... it's actually worse... it's 13.333 Hz. :shock: :oops:
Above, I wrote:
I know it can be easy to get caught-up in the esoteric
Well... maybe I DON'T know! :shock:
joeymorin wrote:
Indeed, there are possibly other factors at work.
:oops: Yeah... like the afore mentioned condition.... :oops:

Thank you Kartman for waking me up! This whole ISR discussion may have been unnecessary, but it is more input toward my overall AVR education. I appreciate all the comments and also thanks to JJ. :wink:

Cris

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

After finishing the tachometer hardware, I had this nagging dissatisfaction with the brightness of the 7 segment displays. I looked for brighter ones but all I found were rated in ucd and the discrete LEDs were rated in mcd. No wonder I was not satisfied with the brightness. :shock: It kept bothering me and I kept trying to convince myself they would be fine. Well, I finally decided to do something about it. So I disassembled the gauge and removed the 7 Segment displays and replaced them with a small PC board with bright LEDs. Here is what it looked like before:

And after... (the PC board at the bottom is what it looked like before the LEDs):

So then I needed to make diffusers like I made for the bar graph of the gauge. Here I'm cutting a pocket (with my CNC) in translucent white acrylic and leaving the horizontal segments of 7 segment numbers:

Below, the vertical segments being cut out. Notice the two pins that are cut on each segment that will be used to locate them properly:

This is what the vertical segments look like ready to place in the pocket with the horizontal segments:

You can see here how the vertical segments fit into the rest of the assembly:

Aluminized mylar is inserted between segments to keep light from bleeding through the small gaps between segments:

Epoxy is poured into the pocket and allowed to cure. Then the diffuser is faced off on top and bottom, and then the perimeter is cut to size.

The new diffuser is then installed into the window of the existing diffuser.

Since I was making new digits, I figured I might as well make them larger. So that means I need to make a new engraved face. Here you see the new one with the old one (the new one is on the left):

This is the tachometer put back together:

The brightness of the old vs new 7 segment display is like night and day! It may even be bright enough for sunlight reading. It saturates the sensor in the camera when taking the above photo.

Cris

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

aeroHAWK wrote:
...an epic tale...

Clearly I should just go back to digging holes for a living... :|

Holy smokes. Nice work! And thanks for documenting it so well.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

joeymorin wrote:
Nice work!
Thanks JJ. Oh, and don't sell yourself short, you have contributed greatly to this gauge! My strength is mechanical hardware which can show up well in photos. Your strength is (I'm assuming) software, which isn't so easy to show in a photo. :wink:

joeymorin wrote:
And thanks for documenting it so well.
I hope by my documenting this, it gives others ideas and incentive so they create something they can be proud of.

Cris

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

All projects have build or buy decisions. I use a ForgeEuropa .3" white seven segment led that hurts my eyes. I cant remember what they cost, maybe a couple bux per digit. But I've never seen anyone make their own displays. I even asked the ForgeEuropa sales dude if they would make a 5x7 dot-matrix white led 5.6mm hi. A dumb display I could refresh with a tiny. They weren't interested. Not enough predicted volume. If you do that, you can have alphanumeric scrolling info on the display. I have most of it written.

Imagecraft compiler user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

bobgardner wrote:
I use a ForgeEuropa .3" white seven segment led that hurts my eyes.
Thanks Bob. I just did a quick search and what I found quickly indicated they are sill only 1/10 the intensity of the LEDs I have. :wink:

Cris

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

aeroHAWK wrote:
I just did a quick search and what I found quickly indicated they are sill only 1/10 the intensity of the LEDs I have. :wink:
Looking forward to seeing a sun-lit shot!

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

joeymorin wrote:
Looking forward to seeing a sun-lit shot!
Don't hold your breath... that sounds like a real difficult photo to get. :shock: (I wish cameras had the fancy digital processing that our eyes have....) :wink: Oh, and I'm in north western Washington... sunshine is not easy to find this time of year.... 8)

Cris

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I've been wrestling with the software PWM for the top segment of the bar graph. I'm looking for assistance in structuring the program.

To bring everyone up to speed... the bar graph has a segment for every 100 RPM. I want to PWM the top segment so if the RPM were 2450, the segment representing 2400 would be lit and the next segment would be 50% duty cycle, for example.

The bar graph uses PCA9922s (shift registers) to drive the LEDs and the 7 segment displays use a MAX7219. There are 5 PCA9922s that are loaded in parallel (using a look up table) and the 7219 uses the USI output. All chips are clocked together using SCK.

I have a routine that outputs all data, and it works great. The issue I have is that I need to output the data when a timer interrupts. Currently it is free running independent of the timer.

As of now, my program structure makes this cumbersome. I planned on making a loop, but I can't seem to figure out how to do it. Here is what I want the loop to do, but the loop is unrolled. Each time data is sent to the 9922s there is an opportunity to leave the top segment on or off. The 7219 needs all 10 bytes to complete the information to refresh all digits.

#define HI7219() PORTB |=  0x01
#define LO7219() PORTB &= ~0x01
#define HI9922() PORTA |=  0x02
#define LO9922() PORTA &= ~0x02
//	Preset MAX7219 Load & Serial Clock bits
#define INITPB() PORTB |=  0x05
//	Set PCA9922 Data bits
#define CLEARA() PORTA &= ~0xf4

// - - - - - - - - - - - - - Send Data to Display - - - - - - - - - - - - - - -

// - -  Wait for timer interrupt here

  LO7219();                       //select MAX7219 chip
  Display(SegsOn, 0x01);          //BarGraf segments : 7219 = 1st digit

// - -  Wait for timer interrupt here

  Display(SegsOn, digit[0]);      //BarGraf segments : 7219 1st digit = digit[0]
  HI7219();                       //latch data in MAX7219

// - -  Wait for timer interrupt here

  LO7219();                       //select MAX7219 chip
  Display(SegsOn, 0x02);          //BarGraf segments : 7219 = 2nd digit

// - -  Wait for timer interrupt here

  Display(SegsOn, digit[1]);      //BarGraf segments : 7219 2nd digit = digit[1]
  HI7219();                       //latch data in MAX7219

// - -  Wait for timer interrupt here

  LO7219();                       //select MAX7219 chip
  Display(SegsOn, 0x03);          //BarGraf segments : 7219 = 3rd digit

// - -  Wait for timer interrupt here

  Display(SegsOn, digit[2]);      //BarGraf segments : 7219 3rd digit = digit[2]
  HI7219();                       //latch data in MAX7219

// - -  Wait for timer interrupt here

  LO7219();                       //select MAX7219 chip
  Display(SegsOn, 0x04);          //BarGraf segments : 7219 = 4th digit

// - -  Wait for timer interrupt here

  Display(SegsOn, digit[3]);      //BarGraf segments : 7219 4th digit = digit[3]
  HI7219();                       //latch data in MAX7219

// - -  Wait for timer interrupt here

  LO7219();                       //MAX7219 load line low
  Display(SegsOn, 0x0a);          //BarGraf segments : 7219 = Set Brightness

// - -  Wait for timer interrupt here

  Display(SegsOn, 0x0f);          //BarGraf segments : 7219 = Brightness to full
  HI7219();                       //MAX7219 load line high

//------------------------------------------------------------------------------

Any suggestions are welcome!

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Naively:

ISR () {
  static uint8_t phase;
  switch (phase) {
    case 0:
      //select MAX7219 chip
      LO7219();                       
      //BarGraf segments : 7219 = 1st digit
      Display(SegsOn, 0x01);          
      break;
    case 1:
      //BarGraf segments : 7219 1st digit = digit[0]
      Display(SegsOn, digit[0]);      
      //latch data in MAX7219
      HI7219();                       
      break;
    case 2:
      //select MAX7219 chip
      LO7219();                       
      //BarGraf segments : 7219 = 2nd digit
      Display(SegsOn, 0x02);          
      break;
    case 3:
      //BarGraf segments : 7219 2nd digit = digit[1]
      Display(SegsOn, digit[1]);      
      //latch data in MAX7219
      HI7219();                       
      break;
    case 4:
      //select MAX7219 chip
      LO7219();                       
      //BarGraf segments : 7219 = 3rd digit
      Display(SegsOn, 0x03);          
      break;
    case 5:
      //BarGraf segments : 7219 3rd digit = digit[2]
      Display(SegsOn, digit[2]);      
      //latch data in MAX7219
      HI7219();                       
      break;
    case 6:
      //select MAX7219 chip
      LO7219();                       
      //BarGraf segments : 7219 = 4th digit
      Display(SegsOn, 0x04);          
      break;
    case 7:
      //BarGraf segments : 7219 4th digit = digit[3]
      Display(SegsOn, digit[3]);      
      //latch data in MAX7219
      HI7219();                       
      break;
    case 8:
      //MAX7219 load line low
      LO7219();                       
      //BarGraf segments : 7219 = Set Brightness
      Display(SegsOn, 0x0a);          
      break;
    case 9:
      //BarGraf segments : 7219 = Brightness to full
      Display(SegsOn, 0x0f);          
      //MAX7219 load line high
      HI7219();                       
      break;
  }
  if (phase == 9) {
    phase = 0;
  }
  else {
    phase++;
  }
}

//--------------------------------------------------------

Untested.

Is Display() a function or a macro? Note that when calling a function from an ISR you run the risk of incurring a lot of overhead, with GCC at least. A look at the generated assembly will tell you how CV handles it.

JJ

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks JJ. I'll be studying your code.

joeymorin wrote:
Is Display() a function or a macro?
This is Display():
//------------------------------------------------------------------------------
//                              OUTPUT TO DISPLAY
//------------------------------------------------------------------------------
void Display(unsigned char SegsOn, unsigned char dataByte)
{
unsigned char clkLO, clkHI;
  clkLO = (1<<USIWM0) |           //set to 3-wire mode
	       (1<<USITC);             //toggle SCK pin
  clkHI = (1<<USIWM0) |           //set to 3-wire mode
	       (1<<USITC)  |           //toggle SCK pin
	       (1<<USICLK);            //shift one bit of USI shift register

  LO9922();                       //PCA9922 load line low

    USIDR = dataByte;             //load USI data register with high byte

  PORTA &= 0x0b;                  //clear PCA9922 data pins
  PORTA |= Table9922[SegsOn][7];  //set PCA9922 data pins
  USICR = clkLO;                  //toggle SCK
  USICR = clkHI;                  //toggle SCK
  PORTA &= 0x0b;                  //clear PCA9922 data pins
  PORTA |= Table9922[SegsOn][6];  //set PCA9922 data pins
  USICR = clkLO;                  //toggle SCK
  USICR = clkHI;                  //toggle SCK
  PORTA &= 0x0b;                  //clear PCA9922 data pins
  PORTA |= Table9922[SegsOn][5];  //set PCA9922 data pins
  USICR = clkLO;                  //toggle SCK
  USICR = clkHI;                  //toggle SCK
  PORTA &= 0x0b;                  //clear PCA9922 data pins
  PORTA |= Table9922[SegsOn][4];  //set PCA9922 data pins
  USICR = clkLO;                  //toggle SCK
  USICR = clkHI;                  //toggle SCK
  PORTA &= 0x0b;                  //clear PCA9922 data pins
  PORTA |= Table9922[SegsOn][3];  //set PCA9922 data pins
  USICR = clkLO;                  //toggle SCK
  USICR = clkHI;                  //toggle SCK
  PORTA &= 0x0b;                  //clear PCA9922 data pins
  PORTA |= Table9922[SegsOn][2];  //set PCA9922 data pins
  USICR = clkLO;                  //toggle SCK
  USICR = clkHI;                  //toggle SCK
  PORTA &= 0x0b;                  //clear PCA9922 data pins
  PORTA |= Table9922[SegsOn][1];  //set PCA9922 data pins
  USICR = clkLO;                  //toggle SCK
  USICR = clkHI;                  //toggle SCK
  PORTA &= 0x0b;                  //clear PCA9922 data pins
  PORTA |= Table9922[SegsOn][0];  //set PCA9922 data pins
  USICR = clkLO;                  //toggle SCK
  USICR = clkHI;                  //toggle SCK

  HI9922();                       //PCA9922 load line high
}

joeymorin wrote:
Note that when calling a function from an ISR you run the risk of incurring a lot of overhead
The ISR only sets a flag.
//******************************************************************************
//********************************* TIMER1 ISR *********************************
//******************************************************************************
#pragma interrupt_handler timer1_ovf_isr:iv_TIM1_OVF
void timer1_ovf_isr(void)
{
  tick++;                         //tick = 780 uSec
  start = 1;                      //set start flag
}

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

What immediately comes to my mind is how long does display() take to execute and is there any advantage to unrolling the loop? My guess is the advantage would be minimal, however, hard numbers talk louder. Then you can estimate how long it would take to update the whole display in one go.
To do your pwm of the bar graph, you only need to update the 9922 with the segment of interest at the timer tick rate - the rest of the display can update at a much lower rate. That gives you the option of spreading the rest of the other display out over a number of timer ticks if you so desire.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
is there any advantage to unrolling the loop?
Thanks Kartman, I guess I didn't make myself clear. What I have is an unrolled loop and I want to make it into a loop. The advantage (I think) would be a more simple and succinct program.
Kartman wrote:
you only need to update the 9922 with the segment of interest at the timer tick rate
The 9922s are loaded in parallel using a look up table. Loading one will load them all. If I want to do one at a time, I have to rewrite the entire subroutine, with the end result of no advantage. The 7219 is also done in parallel with the 9922s. It only takes a few extra clock cycles (5 I think) to toggle the latch and load the USI shift register. I don't see any reason to separate everything since it works well and I get everything in basically the same time it takes for one. :wink:

Cris

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

joeymorin wrote:
A look at the generated assembly will tell you how CV handles it.
Whoops, I misremembered... you're using Imagecraft...

Anyway,

aeroHAWK wrote:
This is Display():
Which would make the ISR a bit too fat, even if the compiler realises it can be inlined (which is why I said "Naively" :))
Quote:
The ISR only sets a flag.
Move the ISR code I posted into it's own function, and call it from main():
  CLI();
  if (start) {
    start = 0;
    SEI();
    DisplayLoop();
  }
  SEI();

DisplayLoop() consists of all the code I previously posted in the ISR, including the static variable phase. That static variable is what lets DisplayLoop() iterate over the ten passes of your 'loop'.

JJ

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

joeymorin wrote:
Whoops, I misremembered... you're using Imagecraft...
It's okay JJ, I don't hold it against you. :wink: (My memory isn't what it used to be.... :shock: )
joeymorin wrote:
Move the ISR code I posted into it's own function
joeymorin wrote:
DisplayLoop() consists of all the code I previously posted in the ISR, including the static variable phase. That static variable is what lets DisplayLoop() iterate over the ten passes of your 'loop'.
Yep, that's what I figured... THANKS JJ!

Your suggestion knocked me off my ineffective thought process and gave me a different perspective that appears to be faster and more elegant! :lol:

Cris

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

aeroHAWK wrote:
knocked me off my ineffective thought process and gave me a different perspective
Probably because early on you started out with a loop.

But then you later added the need to step through one 'pass' of this former loop with each run of an ISR, effectively interleaving each pass with other code. You identified the need to unroll the loop, but perhaps continued to think of it as a loop.

This perhaps made you miss the nature of the new problem that needed solving. When you stop thinking of it as a loop, a simple state machine presents itself as a good solution.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

joeymorin wrote:
Probably because early on you started out with a loop.
Yep, I had tunnel vision. :shock:

Cris

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If you spread the update over 10 ticks, then how are we going to modulate the brightness of one segment? I gather this is the core issue? Or maybe i'm just a couple of steps ahead?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
If you spread the update over 10 ticks, then how are we going to modulate the brightness of one segment?
When calling the Display() subroutine:
  Display(SegsOn, digit[0]);

SegsOn is a variable that dictates the number of bar graph segments that are on. If the first six ticks SegsOn = 25 and the last four ticks SegsOn = 24 segments, the PWM of the top segment is 60% duty cycle. Each tick is 300uS. So each update takes .003 Sec.

It takes 10 ticks to update the bar graph (including top segment PWM).
It takes the same 10 ticks to update the 7219 four digits (including intensity).

I may be misunderstanding your question since this seems so basic, I thought it was obvious. :?

Cris

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Obviously not. So what is the problem that needs to be resolved?
Joey's solution to speeading the load seemed obvious to me, but not to you. So it seems i have some disconnect with the problem you are wanting to solve.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
Joey's solution to spreading the load seemed obvious to me, but not to you.
Exactly! :wink: My inexperience limits the 'tools' in my 'tool belt' so I asked the question half expecting that someone would see an obvious solution that I didn't see. That's what is so great about this forum!
Kartman wrote:
So it seems i have some disconnect with the problem you are wanting to solve.
It may be that you thought it was so basic that it would be obvious to me.... :wink: (The good news is that the problem is now solved) My programming skills are nowhere near as honed as my machining skills :shock: so I may overlook other obvious solutions. :wink: But don't worry, I'll ask here again. 8)

Cris

P.S. If my comments about obviousness offended you, it was not my intention and I apologize.

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ok, now i see you shift the data into the 5 pca9922s in parallel. I had thought you had 5 of them in series. That solves my disconnect. I probably suggested this method sometime ago!
Aren't you lucky i give such good advice.

Edit, you jumped in whilst typing. No offence taken. I can tend to be a tad brief as i steal time in between trains to respond.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
I probably suggested this method sometime ago! Aren't you lucky i give such good advice.
Yes! Very lucky. :wink: Over the last 13 or so months you have provided numerous examples of great advise. But I do want to give credit where credit is due in this case. On the second page of this thread...
Chuck-Rowst wrote:
Instead, drive the 9922's as five parallel 8-bit shift registers - that is with 5 separate data lines, one common clock and one common load line. I believe this approach will offer you a number of advantages.
Kartman wrote:
No offence taken. I can tend to be a tad brief
Okay, good. I know can be 'brief' also.... :wink:

Cris

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

How about ;

static char display_this_digit = 0;
...

// this is what gets called every display refresh

  LO7219();                       //select MAX7219 chip
  Display(SegsOn, display_this_digit + 1 );    //BarGraf segments : select a 7219 digit
  Display(SegsOn, digit[display_this_digit]);    //BarGraf segments : show this 7219 digit
  HI7219();                       //latch data in MAX7219

  display_this_digit ++;
  if ( display_this_digit >= 4 )
    {
    display_this_digit = 0;
// set brightness in 7219
    LO7219();
    Display(SegsOn, 0x0a);
    Display(SegsOn, 0x0f);          //brighness (255=full)
    HI7219();
    };

Re-looping the bit-banging of the PCA9922 bits in Display(,) would make it more understandable to me, even though the compiler will probably unroll it later.

...
  char k;
...
  for ( k = 7; k >= 0; k -- )
      {
      PORTA &= 0x0b;                  //clear PCA9922 data pins
      PORTA |= Table9922[SegsOn][k];  //set PCA9922 data pins
      USICR = clkLO;                  //toggle SCK
      USICR = clkHI;                  //toggle SCK
      };

  HI9922();                       //PCA9922 load line high

As a personal preference I would split the functionality of Display(,) into Display_MAX7219( unsigned char dataByte ) and Display_PCA9922( unsigned char SegsOn ) but that is a minor quibble.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

WOW! Thanks Mike! :wink: It looks like you made it into a loop. :shock: At first glance it looks cool! It'll take some studying on my part to get my head around it. My inexperience makes stuff like this hurts my head until I can digest it. But I like it!

Thanks again!

Cris

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I personally would want to see some comments in the header of display() that outlines the hardware arrangement. Whilst the code is straightforward, the magic numbers and the method hide the magic. As for looping, that would make for less written code but i figured it was splitting hairs. Save a few bytes vs a few cycles.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
I personally would want to see some comments in the header of display() that outlines the hardware arrangement.

https://www.avrfreaks.net/index.p...

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
As for looping, that would make for less written code but i figured it was splitting hairs. Save a few bytes vs a few cycles.
Understood. At this point, it is unclear which way I'd go. But the great thing is that both JJ's and Mike's examples have broadened my thought process. To me, they look obvious once they are described, even though I couldn't see it before.

More 'tools' in my 'tool belt'.... :wink:

Cris

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

UPDATE

 

Oh the wonders of Google!

 

Somebody on the opposite coast (from me) was looking for digital tachometers for a project he is working on. He did a search to see what was available. Lo and behold, when he saw an image of my tachometer (from my posting the project here on AVR Freaks), it struck him as exactly what he wanted!

 

He then registered on this site for the sole purpose of sending me a PM in an effort to acquire some of these. He needs two tachometers plus one spare (three total).

 

We have been corresponding for a while and I expect to get a Purchase Order in the next few days.

 

I know I have said (more than once) that I don't intend this to be a product. This is still the case, as I don't expect to be making enough of them to warrant calling it a product. It is really a custom gauge, programmed specifically for this customers requirements.

 

Thank you again to all of you that contributed to my AVR education! This forum has truly been a great resource for me in my quest to learn about embedded processors.

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

Last Edited: Sat. Sep 27, 2014 - 12:46 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Great news Cris.

 

While "excellence is its own reward", it doesn't hurt the ego to have someone else say so as well.

 

 

Ross McKenzie ValuSoft Melbourne Australia

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

valusoft wrote:

Great news Cris.

 

While "excellence is its own reward", it doesn't hurt the ego to have someone else say so as well.

Well said Ross!

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kudos!

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

SECOND UPDATE

After completing the tachometers mentioned in my first update, I found more applications for this tachometer. Most of the machines in my machine shop have 3 phase A/C motors. My current location does not have 3 phase power available, so I've been using a rotary phase converter. I am now in the process of switching over to Variable Frequency Drives for each machine. This also provides the advantage that I can eliminate the mechanical vari-speed drive system on my milling machine and CNC mill. Here is my manual milling machine before the installation:

 

To eliminate the mechanical speed control, I replaced the monkey motion belts and pulleys with an automotive serpentine belt. I had to make new pulleys and I included a disk with windows for the tachometer:

 

A tachometer display will be very handy for these machines. For my manual milling machine, I decided to use only a digital display (no bar graph) since the machine will be operating at relatively constant RPM. So I made a new display with larger digits and plan to use only the display PCB of the tachometer shown in this thread. This is the back of it before I installed discrete LEDs:

 

The old mechanical speed control is here:

 

I plan to replace it with a new cover where the tachometer display will be along with a linear potentiometer to adjust RPM:

 

This is the VFD installed:

 

The completed electronics and display counting through numbers as a test:

 

This is a test of the system. Yes, it is actually rotating at 1736 RPM but the shutter speed makes the disk appear stationary. The opto-interrupter is difficult to see since the depth of field leaves it out of focus. I did do some tweaking of the software, mostly experimenting with some deadband. Since this is an electric motor, it turns pretty smoothly so with +/- 2 RPM deadband, the display is rock solid. I'm quite pleased.laugh

 

I'll post final pictures when I get the cover finished and display installed....

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. Antoine de Saint-Exupery (1900 - 1944)

Last Edited: Fri. Jan 22, 2016 - 08:21 PM

Pages