Creating a library that contains MsTimer2

Hello world! This is my first post. I'm quite new the arduino and am only playing as a hobby. I bought a 8 Character 14 Segment LED display and have had great fun with 74HC595 logic drivers and with a far old bit of wiring later I have a sketch which drives quite nicely.
This was upgraded to use interupt timers2 via the MsTimer2 library.
I even pushed the "font" array into PROGMEM.
Next step was to develop this as a library.
What I'm trying to do, and wishing Mellis would write the advanced library tutorial, is to wrap/embed the MsTimer2 library within my Library so any sketch need only have the one #include.
Is this doomed to failure? I've out of my depth with errors I'm getting back. :-[

#ifndef c8x14seg_h
#define c8x14seg_h
#define TIMER2_FREQ 3    // Timer2 interupt in milliseconds

#include <avr/interrupt.h>
#include <avr/pgmspace.h>
#include "MsTimer2.h"
#include "WConstants.h"

class c8x14seg {
  public:
    c8x14seg(int dataPin, int clockPin, int latchPin, int ledPin);
    void c8display();

    int bubble[8];  // "alphabet" value of character to display in corresponding bubble.
    int c8x14seg_seconds;
  private:
    int _bub;   // character of the LED and index of bubble array.
    int _segPat;  // hold binary for segments.
    int _dataPin,_clockPin,_latchPin,_ledPin;
    boolean _led; //=HIGH;
    int _interupt_count;
};

#endif
/*
  8x14seg.cpp - 8bubble 14 segment LED driver.
  (c) Simon May Dec 2008

  v0.1  19.12.2008  Simon May  initial version

  This library is free software; you can redistribute it and/or
  modify it under the terms of the GNU Lesser General Public
  License as published by the Free Software Foundation; either
  version 2.1 of the License, or (at your option) any later version.

  This library is distributed in the hope that it will be useful,
  but WITHOUT ANY WARRANTY; without even the implied warranty of
  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  Lesser General Public License for more details.

  You should have received a copy of the GNU Lesser General Public
  License along with this library; if not, write to the Free Software
  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
*/

#include "WProgram.h"
#include <avr/interrupt.h>
#include <avr/pgmspace.h>
#include <MsTimer2.h>
#include <c8x14seg.h>

void c8x14seg::c8x14seg(int dataPin, int clockPin, int latchPin, int ledPin) {
  _dataPin=dataPin;
  _clockPin=clockPin;
  _latchPin=latchPin;
  _ledPin=ledPin;
  pinMode(_dataPin, OUTPUT);
  pinMode(_clockPin, OUTPUT);
  pinMode(_latchPin, OUTPUT);
  if (_ledPin ) pinMode(_ledPin, OUTPUT);
  digitalWrite(_clockPin,LOW);
  digitalWrite(_latchPin,HIGH);
//
  _led=HIGH;
  c8x14seg_seconds=0;
  for (_bub=0;_bub<8;_bub++) bubble[_bub]=0;
  _bub=0;
//
  MsTimer2::set(TIMER2_FREQ,c8display);
  MsTimer2::start();
}

/* ----------
  display
*/
void c8x14seg::c8display(void) {
  _segPat=bubble[_bub];
  digitalWrite(_latchPin,LOW);
  shiftOut(_dataPin,_clockPin,MSBFIRST,(_segPat>>8));
  shiftOut(_dataPin,_clockPin,MSBFIRST,_segPat);
  shiftOut(_dataPin,_clockPin,MSBFIRST,(255 ^ 1<<_bub));
  digitalWrite(_latchPin,HIGH);
  ++_bub%=8;

  if ( (_interupt_count+=TIMER2_FREQ) >= 1000 ) {
    c8x14seg_seconds+=1;
    _interupt_count-=1000;
    if ( _ledPin ) {
      _led=!_led;
      digitalWrite(_ledPin,_led);
    }
  }
}
//fin

and the errors (some of which may well be from my crudely hacked back sketch).

c8x14seg.cpp:28: error: return type specification for constructor invalid
c8x14seg.cpp: In constructor 'c8x14seg::c8x14seg(int, int, int, int)':
c8x14seg.cpp:45: error: argument of type 'void (c8x14seg::)()' does not match 'void (*)()'
hardware\libraries\c8x14seg/c8x14seg.h:7:22: error: MsTimer2.h: No such file or directory


In file included from C:\Documents and Settings\Simon\My Documents\Arduino\arduino-0012\hardware\cores\arduino/WProgram.h:4,


 In function 'void loop()':
 In function 'void show(char*, int, int)':
 In function 'void flip(int)':

I suspect the killer line is MsTimer2::set(TIMER2_FREQ,c8display);
and that I may need to cast this somehow?? :-/

Hello, and welcome, may88!

I'll investigate more later, but the first error I see in the cpp file is:

void c8x14seg::c8x14seg(int dataPin, int clockPin, int latchPin, int ledPin) {

As the message implies, it is illegal for a constructor to return any type -- even "void". Just remove the void to, well, move on to the next error. :slight_smile:

Mikal

To correct the error on line 28, remove the void type. A constructor should never have a type. Note that not specifying a type is different than specifying a void type.

c8x14seg::c8x14seg(int dataPin, int clockPin, int latchPin, int ledPin) {

As for the error on line 45, you can't use c8display that way because it isn't a static member function. Remember that every non-static member function has an implied additional parameter passed on every invocation (called the "this" pointer) that points to the object instance. Static member functions do not have a this pointer.

The error in the include file may be an issue with with the filename case (if you're running on Linux, Mac OSx, etc.). Otherwise, it is an include path issue.

Oh, and for the second error, MSTimer2::set requires for its second argument a pointer to a function taking no arguments and returning "void". You have have passed it a member function taking no arguments and returning "void" -- a subtle but significant difference.

To correct this, rewrite c8display so that it is a global or static member function. Syntactically, this is as easy as adding the keyword "static" in front of the declaration:

static void c8display(void);

although this will introduce other complications as you will no longer have access to any this pointer.

Mikal

My brain! :o

Thanks Mikal and Don for the replies. I had the void in and out a number of times. Sort of thought it was not needed from other examples but then the c8display needs a void.

ok. in the .h I have

  ...
  c8x14seg(int dataPin, int clockPin, int latchPin, int ledPin);
  static void c8display(void);
  ...

and in the .ccp. The compiler did not like static prefix on c8display.

  ...
  c8x14seg::c8x14seg(int dataPin, int clockPin, int latchPin, int ledPin) {
  ...
  void c8x14seg::c8display(void) {
  ...

The other complications being ... I've lost my private variables.

hardware\libraries\c8x14seg/c8x14seg.h: In static member function 'static void c8x14seg::c8display()':
hardware\libraries\c8x14seg/c8x14seg.h:19: error: invalid use of member 'c8x14seg::_segPat' in static member function
c8x14seg.cpp:53: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:15: error: invalid use of member 'c8x14seg::bubble' in static member function
c8x14seg.cpp:53: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:18: error: invalid use of member 'c8x14seg::_bub' in static member function
c8x14seg.cpp:53: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:20: error: invalid use of member 'c8x14seg::_latchPin' in static member function
c8x14seg.cpp:54: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:20: error: invalid use of member 'c8x14seg::_dataPin' in static member function
c8x14seg.cpp:55: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:20: error: invalid use of member 'c8x14seg::_clockPin' in static member function
c8x14seg.cpp:55: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:19: error: invalid use of member 'c8x14seg::_segPat' in static member function
c8x14seg.cpp:55: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:20: error: invalid use of member 'c8x14seg::_dataPin' in static member function
c8x14seg.cpp:56: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:20: error: invalid use of member 'c8x14seg::_clockPin' in static member function
c8x14seg.cpp:56: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:19: error: invalid use of member 'c8x14seg::_segPat' in static member function
c8x14seg.cpp:56: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:20: error: invalid use of member 'c8x14seg::_dataPin' in static member function
c8x14seg.cpp:57: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:20: error: invalid use of member 'c8x14seg::_clockPin' in static member function
c8x14seg.cpp:57: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:18: error: invalid use of member 'c8x14seg::_bub' in static member function
c8x14seg.cpp:57: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:20: error: invalid use of member 'c8x14seg::_latchPin' in static member function
c8x14seg.cpp:58: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:18: error: invalid use of member 'c8x14seg::_bub' in static member function
c8x14seg.cpp:59: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:22: error: invalid use of member 'c8x14seg::_interupt_count' in static member function
c8x14seg.cpp:61: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:16: error: invalid use of member 'c8x14seg::c8x14seg_seconds' in static member function
c8x14seg.cpp:62: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:22: error: invalid use of member 'c8x14seg::_interupt_count' in static member function
c8x14seg.cpp:63: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:20: error: invalid use of member 'c8x14seg::_ledPin' in static member function
c8x14seg.cpp:64: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:21: error: invalid use of member 'c8x14seg::_led' in static member function
c8x14seg.cpp:65: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:21: error: invalid use of member 'c8x14seg::_led' in static member function
c8x14seg.cpp:65: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:20: error: invalid use of member 'c8x14seg::_ledPin' in static member function
c8x14seg.cpp:66: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:21: error: invalid use of member 'c8x14seg::_led' in static member function
c8x14seg.cpp:66: error: from this location
hardware\libraries\c8x14seg/c8x14seg.h:7:22: error: MsTimer2.h: No such file or directory


In file included from C:\Documents and Settings\Simon\My Documents\Arduino\arduino-0012\hardware\cores\arduino/WProgram.h:4,


 In function 'void loop()':
 In function 'void show(char*, int, int)':
 In function 'void flip(int)':

and in the .ccp. The compiler did not like static prefix on c8display.

The static attribute is only needed (and only allowed) in the class definition.

The other complications being ... I've lost my private variables.

Quite so. A static member function is not tied to any instance of the class. It has no this pointer so it can't access any of the instance variables nor any non-static member functions.

It's too bad that whoever designed the MsTimer2::set() member function didn't define the callback function (second parameter) to take a void * parameter. This strategy provides maximum flexibility because the pointer parameter can then point to anything useful to the callback function. Some might object to this strategy because it will cause problems if an incorrect pointer is passed, a point that is well taken. In spite of that drawback, it is an often-used technique.

If it were defined that way, you could have the callback function be a static member function which is passed an instance pointer to an object. Then, that static member function could invoke a non-static member function after casting the void pointer to be a pointer to an MsTimer2 object.

May88 --

Don's observations are all quite correct, and I agree that it's too bad that MsTimer2's callback doesn't have any parametric way to identify some context during the callback.

If -- and this is a big IF -- you intend to create only a single instance of your c8x14seg class, one way to wiggle out of this morass is as follows. (Again, this is slightly kludgy and I am not proud of, but it will work, and my guess is that that is your primary motivation by this time?)

  1. Remove the "static" from c8display(), making it a "normal" member function again:

void c8display();

  1. Towards the top of the .cpp file, just before the constructor definition, declare a pointer to the single instance of your class:

static c8x14seg *pclass = 0;

  1. Create a new (global) callback function that uses that pointer thusly:

void mytimer()
{
if (pclass)
pclass->c8display();
}

  1. At the end of the constructor, set pclass and tell MsTimer2 to use your new callback function:

pclass = this;
MsTimer2::set(TIMER2_FREQ, mytimer);
MsTimer2::start();

Just to repeat, this will break if you intend to create more than one c8x14seg object. But once you get this working, we can work on extending the technique to multiple classes. You could also choose to make mytimer and pclass static members for greater encapsulation.

Mikal

I took a look the MsTimer2 code and it looked different from the Morse libary. The good thing about being out of your depth is there plenty of room for things to go straight over your head.

  1. done
  2. done
  3. done - added a "public" declaration in .h
  4. done.
#ifndef c8x14seg_h
#define c8x14seg_h
#define TIMER2_FREQ 3    // Timer2 interupt in milliseconds

#include <avr/interrupt.h>
#include <avr/pgmspace.h>
#include "MsTimer2.h"
#include "WConstants.h"

class c8x14seg {
  public:
    c8x14seg(int dataPin, int clockPin, int latchPin, int ledPin);
    void c8display(void);
    void mytimer();
    int bubble[8];  // "alphabet" value of character to display in corresponding bubble.
    int c8x14seg_seconds;
  private:
    int _bub;   // character of the LED and index of bubble array.
    int _segPat;  // hold binary for segments.
    int _dataPin,_clockPin,_latchPin,_ledPin;
    boolean _led; //=HIGH;
    int _interupt_count;      

};

#endif
#include "WProgram.h"
#include <avr/interrupt.h>
#include <avr/pgmspace.h>
#include <MsTimer2.h>
#include <c8x14seg.h>

static c8x14seg *pclass = 0;

c8x14seg::c8x14seg(int dataPin, int clockPin, int latchPin, int ledPin) {
  _dataPin=dataPin;
  _clockPin=clockPin;
  _latchPin=latchPin;
  _ledPin=ledPin;
  pinMode(_dataPin, OUTPUT);
  pinMode(_clockPin, OUTPUT);
  pinMode(_latchPin, OUTPUT);
  if (_ledPin ) pinMode(_ledPin, OUTPUT);
  digitalWrite(_clockPin,LOW);
  digitalWrite(_latchPin,HIGH);
//
  _led=HIGH;
  c8x14seg_seconds=0;
  for (_bub=0;_bub<8;_bub++) bubble[_bub]=0;
  _bub=0;
//
  pclass = this;
  MsTimer2::set(TIMER2_FREQ,mytimer);
  MsTimer2::start();
}

/* ----------
  display
*/
void c8x14seg::c8display(void) {
  _segPat=bubble[_bub];
  digitalWrite(_latchPin,LOW);
  shiftOut(_dataPin,_clockPin,MSBFIRST,(_segPat>>8));
  shiftOut(_dataPin,_clockPin,MSBFIRST,_segPat);
  shiftOut(_dataPin,_clockPin,MSBFIRST,(255 ^ 1<<_bub));
  digitalWrite(_latchPin,HIGH);
  ++_bub%=8;

  if ( (_interupt_count+=TIMER2_FREQ) >= 1000 ) {
    c8x14seg_seconds+=1;
    _interupt_count-=1000;
    if ( _ledPin ) {
      _led=!_led;
      digitalWrite(_ledPin,_led);
    }
  }
}

void c8x14seg::mytimer()
{
 if (pclass)
   pclass->c8display();
}

//fin
c8x14seg.cpp: In constructor 'c8x14seg::c8x14seg(int, int, int, int)':
c8x14seg.cpp:48: error: argument of type 'void (c8x14seg::)()' does not match 'void (*)()'
hardware\libraries\c8x14seg/c8x14seg.h:7:22: error: MsTimer2.h: No such file or directory




In file included from C:\Documents and Settings\Simon\My Documents\Arduino\arduino-0012\hardware\cores\arduino/WProgram.h:4,


 In function 'void loop()':
 In function 'void show(char*, int, int)':
 In function 'void flip(int)':

a lot less errors but still it's moaning about the argument type.
-Simon.

Ah, sorry-- make "mytimer" a GLOBAL function, not a member of the class. Or, better yet, since you already have it in the class, declare it "static".

#includeing library headers within headers in Arduino-land can sometimes be problematic. I'd remove that #include <MsTimer2.h> from the .h file. (Not needed, I think?)

Mikal

Ok! I got it to compile by adding "static" to mytimer and making a couple of other fixes to address Arduino bugs:

(library header c8x14seg.h)

#ifndef c8x14seg_h
#define c8x14seg_h
#define TIMER2_FREQ 3    // Timer2 interupt in milliseconds

#include <avr/interrupt.h>
#include <avr/pgmspace.h>
#include "WConstants.h"

class c8x14seg {
  public:
    c8x14seg(int dataPin, int clockPin, int latchPin, int ledPin);
    void c8display(void);
    static void mytimer();
    int bubble[8];  // "alphabet" value of character to display in corresponding bubble.
    int c8x14seg_seconds;
  private:
    int _bub;   // character of the LED and index of bubble array.
    int _segPat;  // hold binary for segments.
    int _dataPin,_clockPin,_latchPin,_ledPin;
    boolean _led; //=HIGH;
    int _interupt_count;

};
#undef int // this little series of #undefs addresses a little bug in Arduino
#undef round
#undef abs
#undef float


#endif

(library code file c8x14seg.cpp)

#include "WProgram.h"
#include <avr/interrupt.h>
#include <avr/pgmspace.h>
#include <MsTimer2.h>
#include <c8x14seg.h>

static c8x14seg *pclass = 0;

c8x14seg::c8x14seg(int dataPin, int clockPin, int latchPin, int ledPin) {
  _dataPin=dataPin;
  _clockPin=clockPin;
  _latchPin=latchPin;
  _ledPin=ledPin;
  pinMode(_dataPin, OUTPUT);
  pinMode(_clockPin, OUTPUT);
  pinMode(_latchPin, OUTPUT);
  if (_ledPin ) pinMode(_ledPin, OUTPUT);
  digitalWrite(_clockPin,LOW);
  digitalWrite(_latchPin,HIGH);
//
  _led=HIGH;
  c8x14seg_seconds=0;
  for (_bub=0;_bub<8;_bub++) bubble[_bub]=0;
  _bub=0;
//
  pclass = this;
  MsTimer2::set(TIMER2_FREQ,mytimer);
  MsTimer2::start();
}

/* ----------
  display
*/
void c8x14seg::c8display(void) {
  _segPat=bubble[_bub];
  digitalWrite(_latchPin,LOW);
  shiftOut(_dataPin,_clockPin,MSBFIRST,(_segPat>>8));
  shiftOut(_dataPin,_clockPin,MSBFIRST,_segPat);
  shiftOut(_dataPin,_clockPin,MSBFIRST,(255 ^ 1<<_bub));
  digitalWrite(_latchPin,HIGH);
  ++_bub%=8;

  if ( (_interupt_count+=TIMER2_FREQ) >= 1000 ) {
    c8x14seg_seconds+=1;
    _interupt_count-=1000;
    if ( _ledPin ) {
      _led=!_led;
      digitalWrite(_ledPin,_led);
    }
  }
}

void c8x14seg::mytimer()
{
 if (pclass)
   pclass->c8display();
}

//fin

and lastly, a sample stub sketch

#include <c8x14seg.h>
#include <MsTimer2.h>

c8x14seg c(1, 2, 3, 4);

void loop(){}
void setup(){}

Good luck!

Mikal

Binary sketch size: 3904 bytes (of a 14336 byte maximum) :o

Thanks again Mikal for your help, patience and utimatey a solution and thanks to Don for your words of wisdom that I wish to understand one day soon.

So,
Global != Public
and I still have some concepts to understand as soon as Mellis writes the next installment of the tutorial. :wink:

One last question. (what is gong on?) The leds now flickers. I'd found that 3ms was the longest delay between updating one of the eight characters (bubbles) on the display so no flicker was visible. The flicker is puzzling as it implies that the interupt is not happening every 3ms. I made "c8display" display just one bubble at a time to I could also tick the c8x14seg_seconds counter.

I put a debug loop in that should show about 4 entries per second and I'm getting 7!?! Is c8display slower ? What would happpen if it was still running when the timer wanted to start another instance?

c8x14seg c8x14seg(12,10,11,13);

void setup() {
  Serial.begin(9600);
   show("may88",1000,1);
}

void loop() {
  Serial.print("bubble[0]=");
  Serial.print(c8x14seg.bubble[0],DEC);  
  Serial.print(";  c8x14seg_seconds=");
  Serial.println(c8x14seg.c8x14seg_seconds,DEC);  
  delay(250);
}
bubble[0]=1334;  c8x14seg_seconds=28
bubble[0]=1334;  c8x14seg_seconds=29
bubble[0]=1334;  c8x14seg_seconds=29
bubble[0]=1334;  c8x14seg_seconds=29
bubble[0]=1334;  c8x14seg_seconds=29
bubble[0]=1334;  c8x14seg_seconds=29
bubble[0]=1334;  c8x14seg_seconds=29
bubble[0]=1334;  c8x14seg_seconds=29
bubble[0]=1334;  c8x14seg_seconds=30

-Simon.

Simon, I'm not certain whether this has anything to do with the symptoms you report, but I notice that _interupt_count is not initialized to 0 in the constructor. You should probably fix that.

The message is apparently printing 7 times per second and not 4, but have you timed it to see if that is really true? My guess is that c8x14seg_seconds is incrementing less frequently than once per second (for reasons I don't immediately see).

Mikal-

ok I'll catch the time but I think the flicker is the dead give away.

in .h

public:
    long unsigned int functime;   // mills() value every cycle.
    long unsigned int difftime;   // mills() value every cycle.
  private:
    long unsigned int _oldtime;   // mills() value every cycle.
    long unsigned int _newtime;   // mills() value every cycle.

in .cpp

c8x14seg::c8x14seg(int dataPin, int clockPin, int latchPin, int ledPin) {
  _oldtime=millis();
}

void c8x14seg::c8display(void) {
  _newtime=millis();   // first line of the function
...
// at bottom of the function.
  difftime=_newtime-_oldtime;  // should indicate the frequency
  _oldtime=_newtime;
  functime=_newtime-millis();   // how long is this function taking.
}

in the sketch

c8x14seg c8x14seg(12,10,11,13);

void setup() {
  Serial.begin(9600);
   show("may88",1,1);
}

void loop() {
   Serial.print("difftime=");
  Serial.print(c8x14seg.difftime,DEC);  
  Serial.print(";  functime=");
  Serial.print(c8x14seg.functime,DEC);  
  Serial.print(";  seconds=");
  Serial.println(c8x14seg.seconds,DEC);  
   delay(250);
}

The Results, to me, imply that the function is quick < 1ms and if being run every 5-7ms not every 3ms. Could something be upsetting the Timer2 function? Have we put it in a place its not happy with?

difftime=5;  functime=0;  seconds=0
difftime=6;  functime=0;  seconds=0
difftime=6;  functime=0;  seconds=0
difftime=6;  functime=0;  seconds=0
difftime=6;  functime=0;  seconds=0
difftime=6;  functime=0;  seconds=0
difftime=6;  functime=0;  seconds=0
difftime=6;  functime=0;  seconds=1
difftime=6;  functime=0;  seconds=1
difftime=6;  functime=0;  seconds=1
difftime=7;  functime=0;  seconds=1
difftime=6;  functime=0;  seconds=1
difftime=6;  functime=0;  seconds=1
difftime=6;  functime=0;  seconds=1
difftime=6;  functime=0;  seconds=1
difftime=6;  functime=0;  seconds=2

Just a couple of observations... first of all, if you're measuring the duration of the function, the correct expression should be millis()-_newtime and not the other way around. But I would expect this to be 0 every time, because your c8display function is an interrupt handler, and as such, during the course of its execution other interrupts, for example the one that helps millis() run, are masked out. I wouldn't rely too heavily, therefore, on what millis() reports.

Mikal

I see alot of ints that could've been chars(/bytes), I think you can shave off some bytes by changing datatype. One byte per changed datatype.

Not a big issue, but if you like you code to be 'optimal', datatypes matters.

Happy arduinoing! (<---where have I seen that before? hmm)

ok, trimmed the size of some of the variable to keep AlphaBeta happy. :wink: In my defence this was first stab, rough and ready code

I did wonder if there might be a problem with interupts when using millis() within an interupt function.

All I can do is count and compare. Same conclusion. The interupt function c8display is not being run as frequently as I think it should be. Hold on... just checking my code for glaring errors before posting, to save further embarrassment :-[
...
in .h added a count.

public:
    long unsigned int msecs;
    long unsigned int count;

in .cpp

c8x14seg::c8x14seg(byte dataPin, byte clockPin, byte latchPin, byte ledPin) {
...
  msecs=0;
  count=0;
...
void c8x14seg::c8display(void) {
  msecs+=TIMER2_FREQ;    // code called every TIMER2_FREQ ms
  count+=1;  // number of times the function has been called.
...

in the sketch: (please excuse the string handling)

long unsigned int newmillis, newmsecs,oldmsecs,oldmillis;
char buff[9];

c8x14seg c8x14seg(12,10,11,13);

void setup() {
  oldmillis=millis();
  oldmsecs=c8x14seg.msecs;
  Serial.begin(9600);
  buff[8]=0;         // null terminate.
}

void loop() {
  newmillis=millis();
  newmsecs=c8x14seg.msecs;
  long unsigned int diffmsecs=newmsecs-oldmsecs;
  long unsigned int diffmillis=newmillis-oldmillis;
  oldmsecs=newmsecs;
  oldmillis=newmillis;
  Serial.print("count=");
  Serial.print(c8x14seg.count,DEC);  
  Serial.print("; diffmsecs=");
  Serial.print(diffmsecs,DEC);  
  Serial.print(";  diffmillis=");
  Serial.print(diffmillis,DEC);  
  Serial.print(";  diffdiff=");
  long unsigned int diffdiff=diffmillis-diffmsecs;
  Serial.println(diffdiff,DEC);  
  int  wee_diff=(int)diffdiff;
  for (i=0;i<8;i++) buff[i]=32;        // space fill
  itoa(wee_diff,&buff[0],10);  
  show(buff,0,1);
  delay(250);
}

The results:

count=0; diffmsecs=0;  diffmillis=0;  diffdiff=0
count=50; diffmsecs=147;  diffmillis=299;  diffdiff=152
count=100; diffmsecs=150;  diffmillis=306;  diffdiff=156
count=151; diffmsecs=153;  diffmillis=307;  diffdiff=154
count=201; diffmsecs=153;  diffmillis=307;  diffdiff=154
count=252; diffmsecs=150;  diffmillis=307;  diffdiff=157
count=303; diffmsecs=153;  diffmillis=309;  diffdiff=156
count=354; diffmsecs=153;  diffmillis=307;  diffdiff=154
count=404; diffmsecs=153;  diffmillis=307;  diffdiff=154
count=455; diffmsecs=150;  diffmillis=307;  diffdiff=157
count=506; diffmsecs=153;  diffmillis=307;  diffdiff=154
count=556; diffmsecs=153;  diffmillis=308;  diffdiff=155
count=607; diffmsecs=150;  diffmillis=307;  diffdiff=157
count=658; diffmsecs=153;  diffmillis=307;  diffdiff=154
count=709; diffmsecs=153;  diffmillis=307;  diffdiff=154
count=759; diffmsecs=153;  diffmillis=307;  diffdiff=154
count=810; diffmsecs=150;  diffmillis=309;  diffdiff=159
count=861; diffmsecs=153;  diffmillis=307;  diffdiff=154
count=911; diffmsecs=153;  diffmillis=307;  diffdiff=154
count=962; diffmsecs=150;  diffmillis=307;  diffdiff=157
count=1013; diffmsecs=153;  diffmillis=307;  diffdiff=154
count=1064; diffmsecs=153;  diffmillis=309;  diffdiff=156
count=1115; diffmsecs=153;  diffmillis=308;  diffdiff=155
count=1165; diffmsecs=153;  diffmillis=308;  diffdiff=155
count=1216; diffmsecs=153;  diffmillis=309;  diffdiff=156
count=1268; diffmsecs=153;  diffmillis=311;  diffdiff=158

diffmillis (unprotected from interupts) is showing the time taken for each loop(). In that time I would have expected diffmsecs to be the same value (give it take an interupt or two). Not so! It's about half the speed. Hence the flicker.

Is this a weird arduino bug? I've tried replacing the definition TIMER2_FREQ with a constant with no change in results. :-?

At 3ms the c8display function is being run about 50 times in 300ms. I'd expected this to be run 100 times. So I can fix the display by changing the TIMER2_FREQ to 1.

TIMER2_FREQ Count (aprox over 300ms) Coments
3ms 50 Half required refresh rate
2ms 75 just too slow - slight flicker
1ms 150 No flicker but with a increased work-load.

Running with TIMER2_FREQ 1 (about every 2ms in relality) cures the flicker at the expense of 50% extra workload (WITHIN AN INTERUPT) which as an extra impact on the performance of the normal code.

I was hoping to use the interupt as a regular tick to increament "seconds" for a millis() rollover workaround.

-simon

I was hoping to use the interupt as a regular tick to increament "seconds" for a millis() rollover workaround.

Simon, I don't understand why you are getting the results you are, but why how would this scheme solve the "rollover" problem?

Mikal

Mikal, my thinking was that the c8display function would be called every 3ms without using millis() so give or take a ms or two the public "seconds" variable would advance every second. I'd make this a long unsigned int and have better control of the rollover.
This is only a working idea and I plan to look at real time clock at some stage.
The long term idea is to develop an alarm clock but to incorporate as many new concepts as my brain allows along the way. The shift registers are the first external ICs I've used. The MsTimer2 was a short cut to using the interrupt functions to drive the display.
Current diversion was/is to place as much of the display functionality in to a library. For tidiness and easy-of-use I was looking to produce a library that contained MsTimer2. The include of includes issue does mess this up a bit but the fact that MsTimer2 misbehaves is more of a bugger.

Current options:

  • Continue to understand/fix the MsTimer2 timings. This is God member territory and beyond my ken, methinks.
  • Drop MsTimer2 from the c8x14seg library. Code this sketch and get it to call the c8display function (if possible).
  • Write my own interrupt code within the c8x14seg library.

I'm off to read up on MsTimer2 and try to get my head around some of the concepts raised earlier in this tread. (callback function, static etc)