Forum Index > Projects > LED Pegboard and Matrix Projects New Topic Post Reply
 Should MeggyJrSimple.h also have a MeggyJrSimple.cpp?
 |  Printable Version
By: Anonymous: spindlenine () on Sunday, May 23 2010 @ 10:38 AM PDT (Read 827 times)  
Anonymous: spindlenine

Hi,

I've been programming with my Meggy Jr. this weekend and I ran in to an issue. I'm a Java programmer by trade with only a little C++ experience, so please excuse my newness and feel free to correct any bad assumptions I may make in this post.

Rather than programming a massive procedural application, I wanted to create a library of classes encapsulating logic for a game. Some of these classes would be responsible for rendering visible game state to the LED board. Since I hadn't written libraries in the Arduino IDE before, I followed their tutorial at http://arduino.cc/en/Hacking/LibraryTutorial.

I was able to get the Morse sample working in the Meggy with a procedural program like this:

PHP Formatted Code

#include <MeggyJrSimple.h>

void setup()
{
  MeggyJrSimpleSetup();
}

void loop()
{
  dot(); dot(); dot();
  dash(); dash(); dash();
  dot(); dot(); dot();
  delay(3000);
}

void dot()
{
  pulse(250);
}

void dash()
{
  pulse(1000);  
}

void pulse(int millis)
{
  DrawPx(3, 4, Red);
  DisplaySlate();
  delay(millis);
 
  ClearSlate();
  DisplaySlate();
  delay(250);
}
 



Expanding this to a simple object-based application, I rewrote the app like so. The main application defines an instance of Morse, and then invokes methods on it. The Morse class itself is responsible for talking to the MeggyJr.

PHP Formatted Code

#include <Morse.h>
#include <MeggyJrSimple.h>

Morse morse;

void setup()
{
  MeggyJrSimpleSetup();
}


void loop()
{
  morse.dot(); morse.dot(); morse.dot();
  morse.dash(); morse.dash(); morse.dash();
  morse.dot(); morse.dot(); morse.dot();
  delay(3000);
}
 



PHP Formatted Code

#ifndef Morse_h
#define Morse_h

#include "WProgram.h"

class Morse
{
  public:
    Morse();
    void dot();
    void dash();
    void pulse(int);
};

#endif
 



PHP Formatted Code

#include "WProgram.h"
#include "Morse.h"
#include <MeggyJrSimple.h>

Morse::Morse()
{
  // nothing
}

void Morse::dot()
{
  Morse::pulse(250);
}

void Morse::dash()
{
  Morse::pulse(1000);  
}

void Morse::pulse(int millis)
{
  Serial.print("Pulsing: ");
  Serial.println(millis);
  DrawPx(3, 4, Red);
  DisplaySlate();
  delay(millis);
 
  ClearSlate();
  DisplaySlate();
  delay(250);
}
 



However, when I try to compile this program, I get tons of errors like the following:


o: In function `SetAuxLEDs(unsigned char)':
/Users/mporges/Documents/code/Arduino/libraries/MeggyJr/MeggyJrSimple.h:119: multiple definition of `SetAuxLEDs(unsigned char)'

/var/folders/Gh/Ghdvnea+H8ev8pGO2BUTok+++TI/-Tmp-/build5130463870809189098.tmp/Morse/Morse.cpp.o:/Users/mporges/Documents/code/Arduino/libraries/MeggyJr/MeggyJrSimple.h:119: first defined here



This seems to be because of the fact that the MeggyJrSimple.h has both the function definitions and implementation in the same file. This doesn't cause any issues in a procedural program where the header is only included once, but in a case like mine where I'm trying to reference the same function definitions in class files, it's causing issues.

With this in mind:

1. Is the approach I'm taking to breaking out the app a sensible one, or am I going to run in to other issues by breaking out my program in to classes?

2. Should the MeggyJrSimple.h file really have an accompanying MeggyJrSimple.cpp implementation in order to get rid of the issues I am seeing when compiling, or was there another reason to do everything in the header that I'm not aware of?

Thanks,

- max



       
  Quote
Back to top
By: Anonymous: spindlenine () on Sunday, May 23 2010 @ 01:19 PM PDT  
Anonymous: spindlenine

Figured I would attempt to port the Meggy library over to the model I was describing with an instance-based approach to the MeggyJrSimple function calls, and I was able to do so. The code is below.

For people who want to use this: note that you should only have one instance of MeggyJrCpp, since this class wraps a reference to the "Meg" function calls. It makes sense that you should only ever have one class messing with Meg in the program at any one time since this is the point of control over the hardware from what I can see from the innards.

I've included the basic finished Morse example using library code over inline calls in the main program; the reference to MeggyJrCpp is injected at runtime. I'm doing this from habit of building IoC-based systems in Java, but you'd probably never have more than one class working with MeggyJrCpp at the same time since you might end up sending mixed signals to the hardware. You do, however, need to init the device during setup() from the main program; trying to call meggyJrSimpleSetup() in the constructor of MeggyJrCpp caused erratic behavior in the hardware.

Main Program

PHP Formatted Code

#include <Morse.h>
#include <MeggyJrCpp.h>

Morse morse;
MeggyJrCpp meggy;

void setup()
{
  // This usually happens here, but if you put the MeggyJrSimple.h include in here, then the compiler complains about multiple definitions
  meggy.meggyJrSimpleSetup();
  morse.meggy = meggy;
  Serial.begin(9600);
}

void loop()
{
  morse.dot(); morse.dot(); morse.dot();
  morse.dash(); morse.dash(); morse.dash();
  morse.dot(); morse.dot(); morse.dot();
  delay(3000);
}
 



Header File (MeggyJrCpp.h)

PHP Formatted Code

#ifndef MeggyJrSimple_h
#define MeggyJrSimple_h
 
#include <MeggyJr.h>

#define MeggyCursorColor   15,15,15             // You can define color constants like this.

// Assign those colors names that we can use
enum colors {
  Dark, Red, Orange, Yellow, Green, Blue, Violet, White,
  DimRed,DimOrange,DimYellow,DimGreen,DimAqua,DimBlue,DimViolet,FullOn,
  CustomColor0, CustomColor1, CustomColor2, CustomColor3, CustomColor4,  
  CustomColor5, CustomColor6, CustomColor7, CustomColor8, CustomColor9
};    

/*
 Sound output:
 
 Frequency is given by the following formula:
 
 f= 8 MHz/divisor, so divisor = 8 MHz/f.   (Round to nearest.)
 Maximum divisor: 65535, so min. frequency is 122 Hz
 
 Example: for 440 Hz, divisor = 18182
*/
// Pre-defined sound divisors
#define ToneB2      64783
#define ToneC3      61157  
#define ToneCs3     57724
#define ToneD3      54485  
#define ToneDs3     51427
#define ToneE3      48541  
#define ToneF3      45816  
#define ToneFs3     43243
#define ToneG3      40816  
#define ToneGs3     38526
#define ToneA3      36363
#define ToneAs3     34323
#define ToneB3      32397
 
#define ToneC4      30578
#define ToneCs4     28862
#define ToneD4      27242
#define ToneDs4     25713
#define ToneE4      24270
#define ToneF4      22908
#define ToneFs4     21622
#define ToneG4      20408
#define ToneGs4     19263
#define ToneA4      18182
#define ToneAs4     17161
#define ToneB4      16198

#define ToneC5      15289
#define ToneCs5     14431
#define ToneD5      13626
#define ToneDs5     12857
#define ToneE5      12135
#define ToneF5      11454
#define ToneFs5     10811
#define ToneG5      10204
#define ToneGs5     9631
#define ToneA5      9091
#define ToneAs5     8581
#define ToneB5      8099

#define ToneC6      7645
#define ToneCs6     7215
#define ToneD6      6810
#define ToneDs6     6428
#define ToneE6      6067
#define ToneF6      5727
#define ToneFs6     5405
#define ToneG6      5102
#define ToneGs6     4816
#define ToneA6      4545
#define ToneAs6     4290
#define ToneB6      4050

#define ToneC7      3822
#define ToneCs7     3608
#define ToneD7      3406
#define ToneDs7     3214
#define ToneE7      3034
#define ToneF7      2863
#define ToneFs7     2703
#define ToneG7      2551
#define ToneGs7     2408
#define ToneA7      2273
#define ToneAs7     2145
#define ToneB7      2025

#define ToneC8      1911
#define ToneCs8     1804
#define ToneD8      1703
#define ToneDs8     1607
#define ToneE8      1517
#define ToneF8      1432
#define ToneFs8     1351
#define ToneG8      1276
#define ToneGs8     1204
#define ToneA8      1136
#define ToneAs8     1073
#define ToneB8      1012

#define ToneC9      956
#define ToneCs9     902
#define ToneD9      851
#define ToneDs9     803

// "Cheater" functions
// NOTE: I think these were put here to make it was to access one-liners without having to wrap them in a method call. I commented them out.
//#define SoundOn()    Meg.SoundState(1)
//#define SoundOff()   Meg.SoundState(0)
#define MakingSound  (TCCR1B > 0)
#define Tone_Update(); {}           // For backwards compatibility.

class MeggyJrCpp
{
    public:
        MeggyJrCpp();

        // TODO: should these be private with accessor functions?
        byte Button_A;      
        byte Button_B;
        byte Button_Up;
        byte Button_Down;
        byte Button_Left;
        byte Button_Right;

        void drawPx(byte xin, byte yin, byte color);
        void displaySlate();
        void clearSlate();
        void meggyJrSimpleSetup();
        void setAuxLEDsBinary(byte n);
        void setAuxLEDs(byte InputLEDs);
        byte readPx(byte xin, byte yin);
        void safeDrawPx(byte xin, byte yin, byte color);
        void editColor(byte WhichColor, byte RedComponent, byte GreenComponent, byte BlueComponent);
        void checkButtonsDown();
        void checkButtonsPress();
        void toneStart(unsigned int divisor, unsigned int duration_ms);

    private:
        MeggyJr Meg; // this needs to be a singleton, so we might want to inject it as an instance var or construct it factory-style
        byte GameSlate[8][8];
        byte lastButtonState;

        static byte ColorTable[26][3];
};

#endif
 



CPP File (MeggyJrCpp.cpp)

PHP Formatted Code

#include <MeggyJrCpp.h>

byte MeggyJrCpp::ColorTable[26][3] =  
    {
      { MeggyDark  },  
      { MeggyRed  }  ,
      { MeggyOrange  },
      { MeggyYellow  },
      { MeggyGreen  },
      { MeggyBlue  } ,
      { MeggyViolet  },
      { MeggyWhite  },  
      { MeggyDimRed  },  
      { MeggyDimOrange  },  
      { MeggydimYellow  },  
      { MeggyDimGreen  },  
      { MeggydimAqua  },  
      { MeggyDimBlue  },  
      { MeggydimViolet  },  
      { MeggyCursorColor},      //Extra bright cursor position color (not white).
      {0,0,0},                  //CustomColor0 (dark, by default)
      {0,0,0},                  //CustomColor1 (dark, by default)
      {0,0,0},                  //CustomColor2 (dark, by default)
      {0,0,0},                  //CustomColor3 (dark, by default)  
      {0,0,0},                  //CustomColor4 (dark, by default)
      {0,0,0},                  //CustomColor5 (dark, by default)
      {0,0,0},                  //CustomColor6 (dark, by default)
      {0,0,0},                  //CustomColor7 (dark, by default)
      {0,0,0},                  //CustomColor8 (dark, by default)
      {0,0,0}                   //CustomColor9 (dark, by default)
    };

MeggyJrCpp::MeggyJrCpp()
{
    // nothing... I attempted init'ing Meggy here by callin meggyJrSimpleSetup(), but the device behaves erratically
    // so I moved the method call back to the instantiating program. I imagine this has something to do with the
    // state of the device prior to setup() being invoked in the main program.
}

// TODO: put this in the constructor?
void MeggyJrCpp::meggyJrSimpleSetup()
{
    Meg = MeggyJr();  
    lastButtonState = Meg.GetButtons();
    Meg.StartTone(0, 0);

    // REPLACED - moved cheater function here since reference to Meg is no longer in header
    // SoundOff();
    Meg.SoundState(0);
}

void MeggyJrCpp::drawPx(byte xin, byte yin, byte color)
{
    GameSlate[xin][yin] = color;
}

// DisplaySlate() :: Write the Game Slate to the Display Memory it.
// This function looks up each color number (name) stored in the Game Slate,
// retreives its R,G,B components from the color table, and writes them to the
// R,G,B parts of the display memory.
void MeggyJrCpp::displaySlate ()
{  
  byte  j = 0;
  while (j < 8)
  {
    Meg.SetPxClr(j, 7, ColorTable[ GameSlate[j][7] ]);  
    Meg.SetPxClr(j, 6, ColorTable[ GameSlate[j][6] ]);  
    Meg.SetPxClr(j, 5, ColorTable[ GameSlate[j][5] ]);  
    Meg.SetPxClr(j, 4, ColorTable[ GameSlate[j][4] ]);  
    Meg.SetPxClr(j, 3, ColorTable[ GameSlate[j][3] ]);  
    Meg.SetPxClr(j, 2, ColorTable[ GameSlate[j][2] ]);  
    Meg.SetPxClr(j, 1, ColorTable[ GameSlate[j][1] ]);  
    Meg.SetPxClr(j, 0, ColorTable[ GameSlate[j][0] ]);  
    j++;
  }      
}

void MeggyJrCpp::clearSlate()
{
  byte i;
  byte j;
  i = 0;
  while (i < 8) {
    j = 0;
    while ( j < 8)
    {
      GameSlate[i][j] = 0;
      j++;
    }
    i++;
  }
}

// Write a byte to the Auxiliary LED set at the top of the LED matrix display.  
// This version reverses bit order, so you can call it with an explicit binary number
void MeggyJrCpp::setAuxLEDsBinary(byte n)
{
    n = (n & 240) >> 4 | (n & 15) << 4;
    n = (n & 204) >> 2 | (n & 51) << 2;
    Meg.AuxLEDs = (n & 170) >>1 | (n & 85) << 1;
}

// Write a byte to the Auxiliary LED set at the top of the LED matrix display.  
void MeggyJrCpp::setAuxLEDs(byte InputLEDs)
{
    Meg.AuxLEDs = InputLEDs;
}

// function to read color of pixel at position (x,y):
byte MeggyJrCpp::readPx(byte xin, byte yin)
{  
    return GameSlate[xin][yin];
}

// Same as above, except checks to see if pixel is onscreen
// This function is new as of v 1.4
void MeggyJrCpp::safeDrawPx(byte xin, byte yin, byte color)
{
    if ((xin >= 0) && (xin <= 7) && (yin >= 0) && (yin <= 7))
    {
        GameSlate[xin][yin] = color;
    }
}

void MeggyJrCpp::editColor(byte WhichColor, byte RedComponent, byte GreenComponent, byte BlueComponent)
{
   ColorTable[WhichColor][0] = RedComponent;
   ColorTable[WhichColor][1] = GreenComponent;
   ColorTable[WhichColor][2] = BlueComponent;
}

void MeggyJrCpp::checkButtonsDown()
{
    byte i = Meg.GetButtons();

    Button_B  = (i & 1);      
    Button_A = (i & 2);    
    Button_Up = (i & 4);
    Button_Down = (i & 8);
    Button_Left = (i & 16);
    Button_Right = (i & 32);

    lastButtonState = i;
}

void MeggyJrCpp::checkButtonsPress()
{
    byte j;
    byte i = Meg.GetButtons();
    j = i & ~(lastButtonState);  // What's changed?

    Button_B  = (j & 1);      
    Button_A = (j & 2);    
    Button_Up = (j & 4);
    Button_Down = (j & 8);
    Button_Left = (j & 16);
    Button_Right = (j & 32);

    lastButtonState = i;
}

void MeggyJrCpp::toneStart(unsigned int divisor, unsigned int duration_ms)
{
  Meg.StartTone(divisor, duration_ms);
}
 



Morse.h

PHP Formatted Code

#ifndef Morse_h
#define Morse_h

#include "WProgram.h"
#include <MeggyJrCpp.h>

class Morse
{
  public:
    Morse();
    void dot();
    void dash();
    void pulse(int);
    MeggyJrCpp meggy;
};

#endif
 



Morse.cpp

PHP Formatted Code

#include "WProgram.h"
#include "Morse.h"
#include <MeggyJrCpp.h>

Morse::Morse()
{
  // nothing
}

void Morse::dot()
{
  Morse::pulse(250);
}

void Morse::dash()
{
  Morse::pulse(1000);  
}

void Morse::pulse(int millis)
{
  Serial.print("Pulsing: ");
  Serial.println(millis);
  meggy.drawPx(3, 4, Violet);
  meggy.displaySlate();
  delay(millis);
 
  meggy.clearSlate();
  meggy.displaySlate();
  delay(250);
}
 



       
  Quote
Back to top
By: Windell (offline) on Sunday, May 23 2010 @ 11:53 PM PDT  
Windell

1. Is the approach I'm taking to breaking out the app a sensible one, or am I going to run in to other issues by breaking out my program in to classes?



It's not the most straightforward way to get the result that you want, but it will work. I'll make my own suggestion a little later.

2. Should the MeggyJrSimple.h file really have an accompanying MeggyJrSimple.cpp implementation in order to get rid of the issues I am seeing when compiling, or was there another reason to do everything in the header that I'm not aware of?



As I believe that we tried to make clear, the simplified library is only there for *simple* programming purposes, and it is set up that way by design. If you need to do anything more advanced, like calling it multiple times from within a library, you should include its functionality-- if you actually need it --in a separate object-oriented format. Also, if you know what you're doing at this level, you may not actually need anything in the simplified library, and you might be better off interacting with the MeggyJr library itself.


Probably the most important function of the simplified library is to explicitly make it so that you *don't* need to use instances of the meggy class-- you never need to say "meggy.drawPx" or anything like that, you can just call the function. For the vast majority of people, this is actually better, since there is only one piece of Meggy hardware, calling multiple instances just doesn't make sense. Moreover, it's much less clear for beginners.

Given that this was the major simplification, I'm not sure how much sense it would make to label your version-- where you've taken that back out --still as a simplified library:: meggy.meggyJrSimpleSetup() is a bit of a contradiction, don't you think?

There is a second reason that many of the functions in the MJSL were not included in the "real" Meggy library, which is that the Arduino software has only recently made it so that libraries were compiled at upload time. Before that, every single library function increased to the code size, whether is was used. Now that this has been fixed, we could roll a couple of the MJSL functions into the main library.



Now, if you do want to implement the same double-buffering and "easy" functions that are part of the MJSL in an object oriented library, that's totally understandable. But, I don't understand why you'd do it only halfway. Why wrap calls, limit it to only one instance, and still have a separate library for those parts? Wouldn't it be a better approach to fold all of the data structures and functions right into the main MeggyJr library and maybe rename it the "expanded" Meggy Jr library?

Windell H. Oskay
drwho(at)evilmadscientist.com
http://www.evilmadscientist.com/
Forum Evil Scientist
Evil Scientist

Status: offline

Registered: 06/15/06
Posts: 1232
Sunnyvale, CA
Profile Email Website  
  Quote
Back to top
By: Anonymous: spindlenine () on Tuesday, May 25 2010 @ 06:37 PM PDT  
Anonymous: spindlenine

Given that this was the major simplification, I'm not sure how much sense it would make to label your version-- where you've taken that back out --still as a simplified library:: meggy.meggyJrSimpleSetup() is a bit of a contradiction, don't you think?

Now, if you do want to implement the same double-buffering and "easy" functions that are part of the MJSL in an object oriented library, that's totally understandable. But, I don't understand why you'd do it only halfway. Why wrap calls, limit it to only one instance, and still have a separate library for those parts? Wouldn't it be a better approach to fold all of the data structures and functions right into the main MeggyJr library and maybe rename it the "expanded" Meggy Jr library?



Windell,

Thanks for the explanation. Although I've been programming a while, I'm a beginner with C++, and that's why the class looks the way it does; I left a lot of the method names the same and kept the logic close to the original so I could incrementally move things around without breaking anything.

You are right that the meggyJrSimpleSetup() call certainly doesn't make much sense, as it would be far more suitable to treat the wrapper class as a gateway and initialize the device either in the constructor or use a factory-style approach to initialize the gateway before returning a reference to it to the main application. The problem is, whenever I tried to initialize the wrapper outside of the setup() function, I got erratic behavior. Do you have any idea why this might be?

My primary goals were to (a) come up with a way to call the simplified library without getting errors for including the header function definitions twice, and (b) create a way to call the simplified library functions against an instance-style class instead of making procedural calls to inline functions. I was able to achieve both goals with the sample code, albeit not in the cleanest fashion. As I start programming my application, I will revisit your comments and see if I can make less of a mess of it as I go! Smile

Cheers,

- max



       
  Quote
Back to top
By: Windell (offline) on Wednesday, May 26 2010 @ 09:58 PM PDT  
Windell

The problem is, whenever I tried to initialize the wrapper outside of the setup() function, I got erratic behavior. Do you have any idea why this might be?



Depends how you do it; if you use the setup()/loop() structure, then the initializations should almost certainly be done in the setup portion. Calling an instance with a fixed name is probably one of those things that you don't want to do more than once. Again, it's a matter of packaging it all up correctly.

Windell H. Oskay
drwho(at)evilmadscientist.com
http://www.evilmadscientist.com/
Forum Evil Scientist
Evil Scientist

Status: offline

Registered: 06/15/06
Posts: 1232
Sunnyvale, CA
Profile Email Website  
  Quote
Back to top
Content generated in: 0.1092 seconds
New Topic Post Reply



 All times are PDT. The time is now 12:44 PM.
Normal Topic Normal Topic
Locked Topic Locked Topic
Sticky Topic Sticky Topic
New Post New Post
Sticky Topic W/ New Post Sticky Topic W/ New Post
Locked Topic W/ New Post Locked Topic W/ New Post
View Anonymous Posts 
Able to Post 
Filtered HTML Allowed 
Censored Content 

Welcome to Evil Mad Scientist Laboratories. New projects are posted on most Wednesdays.


What's New

Stories


Comments last 7 days

Bookmark EMSL

EMSL RSS

Evil Mad Linkblog

Twitter: @EMSL

Facebook page
del.icio.us
feedburner
Feed on Google Reader
YouTube Channel

Subscribe to get new articles by E-mail:

E-mail address:


Preview | Powered by FeedBlitz

My Account





Sign up as a New User
Lost your password?

Who's Online

Guest Users: 20

DIY Hardware for Electronic Art


Interactive LED Panel kits


Meggy Jr RGB
LED matrix game
development kit.


Business-card sized
AVR target boards


Peggy 2
LED Pegboard kits

Forumposts

Order: New Views Posts
Latest 10 Forum Posts
 
Re: Help me creating an arduin..
 By:  Windell
 Wednesday, September 01 2010 @ 07:08 PM PDT
Help me creating an arduino li..
 By:  x86.sll
 Wednesday, September 01 2010 @ 05:22 PM PDT
Re: Fourth Row of Green LED's ..
 By:  Windell
 Sunday, August 29 2010 @ 11:09 AM PDT
Re: Fourth Row of Green LED's ..
 By:  ChrisHale
 Sunday, August 29 2010 @ 08:11 AM PDT
Re: Fourth Row of Green LED's ..
 By:  ChrisHale
 Saturday, August 28 2010 @ 10:56 PM PDT
Re: Fourth Row of Green LED's ..
 By:  Windell
 Saturday, August 28 2010 @ 10:11 PM PDT
Fourth Row of Green LED's Stay..
 By:  ChrisHale
 Saturday, August 28 2010 @ 05:34 PM PDT
Re: Peggy2le serial video ques..
 By:  Windell
 Monday, August 23 2010 @ 11:53 PM PDT
Re: Peggy2le serial video ques..
 By:  milamber
 Monday, August 23 2010 @ 11:47 PM PDT
Re: Multiplexed, Super-long La..
 By:  Windell
 Monday, August 23 2010 @ 08:17 PM PDT