Pages: [1] 2 3   Go Down
Author Topic: What am doing wrong here with my class?  (Read 1423 times)
0 Members and 1 Guest are viewing this topic.
Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
class LED {
 
  static byte MODULES; // Number of LED modules connected.  Defined in config.txt.
 
  // Can't declare size of this because number of LED modules can vary.
  //static byte data[LEDMODULES*36] = {0}; // LED data is stored as a string of 12 bit LED values.  There are 3 bytes for every two LEDs.  We store it this way because the data does not need to be altered when we are shifting it out to the TLC5947s.  Modifying the data with bit shifts would be very expensive and there is a lot of data to move.
  static byte * scale; // Scaling factor for LED brightness.  Used when say, you want to be able to adjust the brightness of the powercell without changing all the code that assumes the max desired brightness is 1.0.  Value stored here is scaling_factor*255, where scaling factor is 0..1.
 
  public:

    void set(int, float, float); 
    float get(int);
   
  private:

};

byte LED::MODULES = 2; // Eventually this will be defined in config.txt.
byte * LED::scale = new byte [LED::MODULES * 24]; //{255};

//for (int x = 0; x < (LED::MODULES * 24); x++) { LED::scale[x] = 255; }



I haven't used classes in C++ before and I'm trying to create a dynamic array of bytes, but the compiler complains about the for loop - "expected unqualified-id before for", and when I comment that out it says "undefined reference to operator new [] (unsigned int)".

Also, for some weird reason if I remove the word "byte" from in front of LED::MODULES = 2, such that LED is now in the first column of the IDE, LED is written in bold, but if I add a single space in front, it's written normally again.
Logged

Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Another question I have is why do I need byte in front of this?
byte LED::MODULES = 2;

I mean if I can access LED::MODULES outside of the class wherever I want why do I need the type identifier there at all?
Logged

Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Well I guess I can do this instead:

Code:
class LED {
 
  static byte MODULES; // Number of LED modules connected.  Defined in config.txt.
 
  // Can't declare size of this because number of LED modules can vary.
  //static byte data[LEDMODULES*36] = {0}; // LED data is stored as a string of 12 bit LED values.  There are 3 bytes for every two LEDs.  We store it this way because the data does not need to be altered when we are shifting it out to the TLC5947s.  Modifying the data with bit shifts would be very expensive and there is a lot of data to move.
  static byte* scale; // Scaling factor for LED brightness.  Used when say, you want to be able to adjust the brightness of the powercell without changing all the code that assumes the max desired brightness is 1.0.  Value stored here is scaling_factor*255, where scaling factor is 0..1.
 
  public:

    void set(int, float, float); 
    float get(int);
   
  private:

};

byte LED::MODULES = 2; // Eventually this will be defined in config.txt.
byte * LED::scale = (byte *) malloc(LED::MODULES * 24 * sizeof(byte));

That works.  I'm not sure why the other method doesn't work.  Maybe the Arduino environment doesn't support C++.  But if not, I don't understand why it supports classes.
Logged

United Kingdom
Offline Offline
Tesla Member
***
Karma: 224
Posts: 6593
Hofstadter's Law: It always takes longer than you expect, even when you take into account Hofstadter's Law.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

I haven't used classes in C++ before and I'm trying to create a dynamic array of bytes, but the compiler complains about the for loop - "expected unqualified-id before for", and when I comment that out it says "undefined reference to operator new [] (unsigned int)".

Statements (such as your for-loop) can only be used inside functions. When did you want the for-loop to be executed? If you want it to be executed when an object of the class is constructed, put it inside the constructor:

Code:
class LED {
  
  static byte MODULES; // Number of LED modules connected.  Defined in config.txt.
  
  // Can't declare size of this because number of LED modules can vary.
  //static byte data[LEDMODULES*36] = {0}; // LED data is stored as a string of 12 bit LED values.  There are 3 bytes for every two LEDs.  We store it this way because the data does not need to be altered when we are shifting it out to the TLC5947s.  Modifying the data with bit shifts would be very expensive and there is a lot of data to move.
  static byte * scale; // Scaling factor for LED brightness.  Used when say, you want to be able to adjust the brightness of the powercell without changing all the code that assumes the max desired brightness is 1.0.  Value stored here is scaling_factor*255, where scaling factor is 0..1.
 
  public:
    LED();
    void set(int, float, float);  
    float get(int);
    
  private:

};

byte LED::MODULES = 2; // Eventually this will be defined in config.txt.
byte * LED::scale = new byte [LED::MODULES * 24]; //{255};

LED::LED()
{
  for (int x = 0; x < (LED::MODULES * 24); x++) { LED::scale[x] = 255; }
}

Do you realise that if you create more than on LED object, they will all share the same MODULES and scale variables? I don't think that's what you want. Try this:

Code:
class LED {
  byte numModules; // Number of LED modules connected.  Defined in config.txt.
  byte *data; // LED data is stored as a string of 12 bit LED values.  There are 3 bytes for every two LEDs.  We store it this way because the data does not need to be altered when we are shifting it out to the TLC5947s.  Modifying the data with bit shifts would be very expensive and there is a lot of data to move.
  byte * scale; // Scaling factor for LED brightness.  Used when say, you want to be able to adjust the brightness of the powercell without changing all the code that assumes the max desired brightness is 1.0.  Value stored here is scaling_factor*255, where scaling factor is 0..1.
 
  public:
    LED(byte nModules);
    void set(int, float, float);  
    float get(int) const;
};

LED::LED(byte nModules)
{
  numModules = nModules;
  data = new byte[nModules * 36];
  scale = new byte [nModules * 24];
  for (int x = 0; x < (nModules * 24); x++)
  {
    scale[x] = 255;
  }
}

Also, for some weird reason if I remove the word "byte" from in front of LED::MODULES = 2, such that LED is now in the first column of the IDE, LED is written in bold, but if I add a single space in front, it's written normally again.

That sounds like a quirk of the Arduino IDE to me.
Another question I have is why do I need byte in front of this?
byte LED::MODULES = 2;

I mean if I can access LED::MODULES outside of the class wherever I want why do I need the type identifier there at all?

Because that construct is a static initializer, not an assignment.
« Last Edit: November 04, 2012, 04:50:26 am by dc42 » Logged

Formal verification of safety-critical software, software development, and electronic design and prototyping. See http://www.eschertech.com. Please do not ask for unpaid help via PM, use the forum.

Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Do you realise that if you create more than on LED object, they will all share the same MODULES and scale variables? I don't think that's what you want.

Actually, that's exactly what I want.  I'm just using the class as a convenient way to encapsulate my functions to keep everything tidy.  I don't have any intention of creating any LED objects at all.

Previously I had a global array and I had functions getLED and setLED to get and set them at a bit level with integer values.  I was going to place these within the class as private members and have the public get and set methods which operate using floats and are 1-indexed and take into account a scaling factor.

I'm not too familiar with classes in C++, but I previously have used BlitzMax which had similar constructs, so I understand the concepts, but the syntax is getting me.

     
Logged

Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I'm now having a new problem.  None of my LEDs are lighting up, and I have no idea why.  I suspect something has gone screwy with my pointers.

Previously I had declared my array like so:

Code:
//const int LEDMODULES = 2; // 2 is very good.  3 is good.  4 is a bit choppy.  6 is very choppy.  8 doesn't work.  Changing FPS doesn't really help with this.

//uint8_t LEDData[LEDMODULES*36] = {0}; // LED data is stored as a string of 12 bit LED values.  There are 3 bytes for every two LEDs.  We store it this way because the data does not need to be altered when we are shifting it out to the TLC5947s.  Modifying the data with bit shifts would be very expensive and there is a lot of data to move.  

And I accessed it like so:

Code:
/*
This function sets the value of the LED at the specified index. (value = 0..4095 - 0 is first index)
*/
void setLED(int index, int value) { // Could change this to take a floating point argument!  Also may want to change so index 1 is first LED.

  int firstBit, firstByte;

  // index * 12 = first bit of LED value.
  // (index * 12) / 8 = byte containing first bit.
  
  // Format of data:
  // Bit order -> 76543210
  // [5]=33333333 [4]=33332222 [3]=2222222 [2]=11111111 [1]=11110000 [0]=00000000 <- Array starts here.
  
  // Note that the data is shifted out starting with the last byte in the array and moving back to the first, and it is shifted out in MSB order.
  
  // (index * 12) / 8 = start byte
  // (0 * 12) / 8 = 0
  // (1 * 12) / 8 = 1
  // (index * 12) % 8 = start bit (will be 0 or 4)
  //( 1 * 12) % 8 = 4
  
  firstBit = index * 12;  
  firstByte = firstBit / 8;  
    
  if ((firstBit % 8) == 0) { // LEDs 0, 2, 4...

    LEDData[firstByte] = value; // Place bits 7..0 in first byte.
    LEDData[firstByte+1] = (LEDData[firstByte+1] & 0b11110000) | (value >> 8);   // Place bits 11..8 in bits 3..0 of second byte.  
      
  } else { // LEDs 1, 3, 5...
  
    LEDData[firstByte] = (LEDData[firstByte] & 0b00001111) | ((value & 0b1111) << 4); // Place bits 3..0 in bits 7..4 of first byte.
    LEDData[firstByte+1] = value>>4; // Place bits 11..4 in second byte.    
    
  }
  
}


And like so:

Code:
/*
This function shifts all the LED data out to the TLC5947's.
*/

#define NOP __asm__ __volatile__ ("nop\n");
#define WAIT NOP NOP NOP NOP NOP NOP NOP NOP NOP NOP  // 10 NOPs

void updateLEDs() {
 
  const int LEDFrameTime = 1000 / 30; // The number of milliseconds between each update of the LEDs.  1000 / 24 = 24fps.
  
  static unsigned long nextLEDFrameTime = 0;  
  
  uint8_t *thisLED;
  uint8_t *lastLED;

  if (time >= nextLEDFrameTime) { // If it is time for an LED update...  
  
    //unsigned int time = micros();
      
    thisLED = &LEDData[LEDMODULES*36]; // The first operation is to decrement this pointer, so the first time we write thisLED it will be array index LEDMODULES*36-1
    lastLED = &LEDData[0];
  
    cli(); // Halt interrupts.
        
    do {      
    
      SPDR = *--thisLED; WAIT;            
      //while (!(SPSR & _BV(SPIF))); SPDR = *thisLED--; // Wait for transfer of byte over SPI bus to complete, then transfer *thisLED into SPDR register, and decrement address of *thisLED.
          
    } while (thisLED != lastLED); // thisLED is decremented one last time before we hit the end of the loop, so after byte 0 transfers, the loop exits.

    WAIT; // Wait for last byte to finish transfer.
    SPSR = SPSR & ~_BV(SPIF); // Clear transfer flag.
    
    //while (!(SPSR & _BV(SPIF))); // Wait for last byte to finish transfer.
    
    // 15 nops and:
    // 6x unroll = 276us
    // 12x unroll = 256us
    // 24x unroll = 252us
    
    // This may be leaving the SPIF flag set.  
    
    // Clear SPIF flag:
    //while (!(SPSR & _BV(SPIF))); // Wait for last byte to finish transfer.

    // 340 microseconds
    // The trick which makes the above so fast is that we decrement the pointer and do the conditional jump while we are waiting for the previous byte transfer to complete.

    // Move data from shift register into latch register:
    
      LATPORT &= ~(1 << LATPIN); // Set LATCH pin low.  It is important to do this first, just before transitioning high!  I don't know why, but the LED modules won't work right otherwise!
      LATPORT |= (1 << LATPIN); // Set LATCH pin high.  Transitioning from low to high moves all data from the shift register into the latch registers.

      ENAPORT |= (1 << ENAPIN); // Set BLANK pin high. Turn all outputs off, and reset grayscale timer.  Blanking reduces flicker, but it does not seem to matter if it is before, after, or in the middle of latching.
      ENAPORT &= ~(1 << ENAPIN); // Set BLANK pin low. Turn all outputs on.
      
    sei(); // Reenable interrupts.
    
    //time = micros() - time;
    //Serial1.println(time,DEC);
    
    nextLEDFrameTime = time + LEDFrameTime; // Set the time of the next update.
    
  }
}


Logged

Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset



Now however, I've changed the code to this, and it's not working:

Code:
class LED {
 
  public:
 
    static int MODULES;   // Number of LED modules connected.  Defined in config.txt.
 
    static byte * data;    // LED data is stored as a string of 12 bit LED values.  There are 3 bytes for every two LEDs.  We store it this way because the data does not need to be altered when we are shifting it out to the TLC5947s.  Modifying the data with bit shifts would be very expensive and there is a lot of data to move.
    static byte * scale;   // Scaling factor for LED brightness.  Used when say, you want to be able to adjust the brightness of the powercell without changing all the code that assumes the max desired brightness is 1.0.  Value stored here is scaling_factor*255, where scaling factor is 0..1.
 
    void set(int, float, float); 
    float get(int);
   
  private:

};

// Class global variable declatations:
  int LED::MODULES;
  byte * LED::data;
  byte * LED::scale;

Inside setup():

Code:
  // Configure LEDs:
   
      LED::MODULES = 2; // Eventually this will be defined in config.txt.

      LED::data = (byte *) malloc(LED::MODULES*36 * sizeof(byte));
      LED::scale = (byte *) malloc(LED::MODULES*24 * sizeof(byte));
   
      for (int x = 0; x < (LED::MODULES * 36); x++) { LED::data[x] = 0; }
      for (int x = 0; x < (LED::MODULES * 24); x++) { LED::scale[x] = 255; }  // Set LED brightness scale factor to default, then adjust only those which are defined in config.txt.

Code:
/*
This function sets the value of the LED at the specified index. (value = 0..4095 - 0 is first index)
*/
void setLED(int index, int value) { // Could change this to take a floating point argument!  Also may want to change so index 1 is first LED.

  int firstBit, firstByte;

  // index * 12 = first bit of LED value.
  // (index * 12) / 8 = byte containing first bit.
 
  // Format of data:
  // Bit order -> 76543210
  // [5]=33333333 [4]=33332222 [3]=2222222 [2]=11111111 [1]=11110000 [0]=00000000 <- Array starts here.
 
  // Note that the data is shifted out starting with the last byte in the array and moving back to the first, and it is shifted out in MSB order.
 
  // (index * 12) / 8 = start byte
  // (0 * 12) / 8 = 0
  // (1 * 12) / 8 = 1
  // (index * 12) % 8 = start bit (will be 0 or 4)
  //( 1 * 12) % 8 = 4
 
  firstBit = index * 12; 
  firstByte = firstBit / 8; 
   
  if ((firstBit % 8) == 0) { // LEDs 0, 2, 4...

    LED::data[firstByte] = value; // Place bits 7..0 in first byte.
    LED::data[firstByte+1] = (LED::data[firstByte+1] & 0b11110000) | (value >> 8);   // Place bits 11..8 in bits 3..0 of second byte.   
     
  } else { // LEDs 1, 3, 5...
 
    LED::data[firstByte] = (LED::data[firstByte] & 0b00001111) | ((value & 0b1111) << 4); // Place bits 3..0 in bits 7..4 of first byte.
    LED::data[firstByte+1] = value>>4; // Place bits 11..4 in second byte.     
   
  }
 
}

Code:
/*
This function shifts all the LED data out to the TLC5947's.
*/

#define NOP __asm__ __volatile__ ("nop\n");
#define WAIT NOP NOP NOP NOP NOP NOP NOP NOP NOP NOP  // 10 NOPs

void updateLEDs() {
 
  const int LEDFrameTime = 1000 / 30; // The number of milliseconds between each update of the LEDs.  1000 / 24 = 24fps.
 
  static unsigned long nextLEDFrameTime = 0;   
 
  //uint8_t *thisLED;
  //uint8_t *lastLED;

  byte *thisLED;
  byte *lastLED;

  if (time >= nextLEDFrameTime) { // If it is time for an LED update... 
 
    //unsigned int time = micros();
     
    thisLED = &LED::data[LED::MODULES*36]; // The first operation is to decrement this pointer, so the first time we write thisLED it will be array index LEDMODULES*36-1
    lastLED = &LED::data[0];
   
    cli(); // Halt interrupts.
       
    do {       
   
      SPDR = *--thisLED; WAIT;           
      //while (!(SPSR & _BV(SPIF))); SPDR = *thisLED--; // Wait for transfer of byte over SPI bus to complete, then transfer *thisLED into SPDR register, and decrement address of *thisLED.
         
    } while (thisLED != lastLED); // thisLED is decremented one last time before we hit the end of the loop, so after byte 0 transfers, the loop exits.

    WAIT; // Wait for last byte to finish transfer.
    SPSR = SPSR & ~_BV(SPIF); // Clear transfer flag.
   
    //while (!(SPSR & _BV(SPIF))); // Wait for last byte to finish transfer.
   
    // 15 nops and:
    // 6x unroll = 276us
    // 12x unroll = 256us
    // 24x unroll = 252us
   
    // This may be leaving the SPIF flag set. 
   
    // Clear SPIF flag:
    //while (!(SPSR & _BV(SPIF))); // Wait for last byte to finish transfer.

    // 340 microseconds
    // The trick which makes the above so fast is that we decrement the pointer and do the conditional jump while we are waiting for the previous byte transfer to complete.

    // Move data from shift register into latch register:
   
      LATPORT &= ~(1 << LATPIN); // Set LATCH pin low.  It is important to do this first, just before transitioning high!  I don't know why, but the LED modules won't work right otherwise!
      LATPORT |= (1 << LATPIN); // Set LATCH pin high.  Transitioning from low to high moves all data from the shift register into the latch registers.

      ENAPORT |= (1 << ENAPIN); // Set BLANK pin high. Turn all outputs off, and reset grayscale timer.  Blanking reduces flicker, but it does not seem to matter if it is before, after, or in the middle of latching.
      ENAPORT &= ~(1 << ENAPIN); // Set BLANK pin low. Turn all outputs on.
     
    sei(); // Reenable interrupts.
   
    //time = micros() - time;
    //Serial1.println(time,DEC);
   
    nextLEDFrameTime = time + LEDFrameTime; // Set the time of the next update.
   
  }
}


Where have I gone wrong here?
Logged

Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Because that construct is a static initializer, not an assignment.

Well it seems weird that I should have to tell the compiler twice that my variable is a byte.  I guess it has something to do with putting the class definition in a header and maybe it's so you can create the variable from inside a function and have it only in that scope, but that just seems weird to me.  I can't imagine why you would want a class definition inside a header file with statics in it that can only be accessed from one function, and what happens if you attempt to define them from within multiple functions?  Seems messy.

When I was doing this stuff in Blitzmax, in a class definition if you created a variable like that it just existed and you could access it from anywhere if it was public.  And you could initialize it to whatever you wanted without having to have it be constant.  Seemed pretty logical and simple to me.
Logged

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 474
Posts: 18696
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

This compiles, I'm not sure I particularly like it:

Code:
void* operator new [] (size_t size) { return malloc (size); }
void operator delete [] (void * p) { free (p); }

class LED {

  public:
 
  static byte * scale;
  const static byte MODULES;

  void set(int, float, float); 
  float get(int);

};

const byte LED::MODULES = 2; // Eventually this will be defined in config.txt.
byte * LED::scale ;  // instantiate the static member

void setup ()
  {
  LED::scale = new byte [LED::MODULES * 24];
  }  // end of setup
 
void loop ()
  {
  }  // end of loop

Logged

Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

My code compiles now, it just doesn't run properly.  (See a couple posts above with before and after code.)

And I'm still wondering why new doesn't seem to work.  I see you had to recreate it yourself to get the code you wrote to compile.
Logged

United Kingdom
Offline Offline
Tesla Member
***
Karma: 224
Posts: 6593
Hofstadter's Law: It always takes longer than you expect, even when you take into account Hofstadter's Law.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Because that construct is a static initializer, not an assignment.

Well it seems weird that I should have to tell the compiler twice that my variable is a byte.  I guess it has something to do with putting the class definition in a header and maybe it's so you can create the variable from inside a function and have it only in that scope, but that just seems weird to me.  I can't imagine why you would want a class definition inside a header file with statics in it that can only be accessed from one function, and what happens if you attempt to define them from within multiple functions?  Seems messy.

When I was doing this stuff in Blitzmax, in a class definition if you created a variable like that it just existed and you could access it from anywhere if it was public.  And you could initialize it to whatever you wanted without having to have it be constant.  Seemed pretty logical and simple to me.

1. You didn't declare that variable public, you declared it in a class prior to the first instance of "public:", "private" or "protected". So it's private by default.

2. Initialization and assignment are very different concepts in C++. You are using the term 'initialize' when you mean 'assign'. You can initialize a variable exactly once, at the start of its lifetime. You can assign it as many times as you want to, subject to accessibility, const-qualification, etc.
Logged

Formal verification of safety-critical software, software development, and electronic design and prototyping. See http://www.eschertech.com. Please do not ask for unpaid help via PM, use the forum.

Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

So nobody knows why the pointers to my data are no longer working?  The stuff with new I've given up on for the moment.  I used malloc instead, it compiles, I'm happy with that for now.  But I haven't got a clue why my pointers aren't working anymore.
Logged

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 474
Posts: 18696
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Can you define "no longer working"? Do you mean it crashes? What does it do compared to what you expect?

Your usage of the class is, ah, different to how classes are normally used.

I don't understand why some, if not all, of the procedural code for the class is done outside it, eg. here:

Quote
Inside setup():

Code:
 // Configure LEDs:
    
      LED::MODULES = 2; // Eventually this will be defined in config.txt.

      LED::data = (byte *) malloc(LED::MODULES*36 * sizeof(byte));
      LED::scale = (byte *) malloc(LED::MODULES*24 * sizeof(byte));
    
      for (int x = 0; x < (LED::MODULES * 36); x++) { LED::data[x] = 0; }
      for (int x = 0; x < (LED::MODULES * 24); x++) { LED::scale[x] = 255; }  // Set LED brightness scale factor to default, then adjust only those which are defined in config.txt.

Since the class only has static members there doesn't seem to be any point in having a class.

Quote
And I'm still wondering why new doesn't seem to work.  I see you had to recreate it yourself to get the code you wrote to compile.

They omitted new and delete from the earlier distributions. It is in the current one but not new[] and delete[] so I had to create them, yes. I think I have a bug report in about that.

It may not matter too much in your case, but when working with class objects you should use new and not malloc. In your case the only thing you are doing new on is a byte* so it won't make a lot of difference.
Logged

Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Can you define "no longer working"? Do you mean it crashes? What does it do compared to what you expect.

The code seems to still run (My sound is working, and I can push a button and trigger something on a port) but my LEDs no longer light up.  Since I know the function to update the LEDs is being called and I'm not getting flickering, that means the modules are being sent a stream of 0's.  If they were being sent garbage they would flicker.  And if they were being sent nothing, they would flicker as well because the sound update sends data down that bus and if I hook up more led modules than data is sent each update then on the remaining modules I get garbage. 

So for some reason this code is not working:

Code:
    thisLED = &LED::data[LED::MODULES*36]; // The first operation is to decrement this pointer, so the first time we write thisLED it will be array index LEDMODULES*36-1
    lastLED = &LED::data[0];
   
    cli(); // Halt interrupts.
       
    do {       
   
      SPDR = *--thisLED; WAIT;           
      //while (!(SPSR & _BV(SPIF))); SPDR = *thisLED--; // Wait for transfer of byte over SPI bus to complete, then transfer *thisLED into SPDR register, and decrement address of *thisLED.
         
    } while (thisLED != lastLED); // thisLED is decremented one last time before we hit the end of the loop, so after byte 0 transfers, the loop exits.

    WAIT; // Wait for last byte to finish transfer.
    SPSR = SPSR & ~_BV(SPIF); // Clear transfer flag.
« Last Edit: November 04, 2012, 03:35:15 pm by scswift » Logged

Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Your usage of the class is, ah, different to how classes are normally used.
Since the class only has static members there doesn't seem to be any point in having a class.

I figured using a class to have:
LED::set()
LED::get()
SERVO::set()
SERVO::get()

And having those encapsulated in their own class with their own private static data (the data array is only public at the moment because I haven't finished moving the other LED functions that actually set that up into the class), was better than having a bunch of global variables and just naming my functions:

LEDset()
SERVOget()

Also I figured it was about time that I learned C++ class syntax since for years I avoided OOP and stuck with C, but I started using it quite heavily once I learned Blitzmax which I understand has a lot of similarities to C++ (and a lot of differences).

It wouldn't make sense to say create an array of LED objects, when all an LED is a brightness and a scaling factor and I need to be able to blast the data out to the led modules as fast as possible so the data has to be kept in a packed array of 12 bit numbers.


Quote
I don't understand why some, if not all, of the procedural code for the class is done outside it, eg. here:

I put that code outside the class because the tutorials I found online said that is what you need to do if you want to set static variables in a class to a default value.  Since I am not creating any LED objects, how else would that code get called?  I understand there is a function that gets called when you create the first object but I'm not creating any.
Logged

Pages: [1] 2 3   Go Up
Jump to: