bartoszbielawski / LEDMatrixDriver

A replacement for Arduino's LedControl library
MIT License
75 stars 25 forks source link

Module order problem #21

Closed quakec closed 5 years ago

quakec commented 5 years ago

I have a single board hosting 4 MAX7219 chips and LED matrices, using this driver is much faster BUT the order in which the modules are addressed are backwards. Using the included marquee example, module[0] is the first to draw the text (as it is brought into display), module[1] is the next to be drawn on and so on... the result is as you can imagine; you get a scroll on number one, as it disappears it appears on the next one from the wrong side. To resolve this, it should start drawing on module[3], then [2] and so on. How do I address and resolve this problem using your driver?

bartoszbielawski commented 5 years ago

I'm away from my computer at the moment but there's the address calculation in setPixel method. There's the 7 - (x & 7) expression. Changing it to x & 7 may fix the problem. There's no other way to change the behaviour at the moment. I may add a define for it if needed.

quakec commented 5 years ago

Thanks for replying so quickly! :)

I'll try your change soon; it looks like I need to rotate my display 180 degrees but thats perfectly fine :)

bartoszbielawski commented 5 years ago

Yes, rotation may also be needed. In general it's hard to hardcore all the ways the PCBs can be done... So the addressing itself can be changed to bend it the way you need it. :)

quakec commented 5 years ago

I honestly feel like I have bought a board, which the chinese would have created as a "joke" putting everything backwards, the original arduino ledcontrol driver has this setRow() function, faster because it can set the LEDs using a single byte, if you use setCol() it is ridiculously slow. But surely you would want to setCol if you are scrolling...

bartoszbielawski commented 5 years ago

I'm actually buffering all the writes. The original version writes the whole row every time you update a single pixel.

The order of modules and orientation can be truly random :)

quakec commented 5 years ago

yes I noticed, good job! The key is understanding the hardware :) Perhaps this driver should have some orientation options which are exposed to instatiation method, instead of just the scroll function :)

bartoszbielawski commented 5 years ago

The problem is that if I pass an enumeration specifying layout it will execute the switch for each of the pixels while it can't really change. I'd prefer to configure this during compilation. Maybe an address translation function...

You can also check what is available in the API from Adafruit GFX base class.

quakec commented 5 years ago

I'm away from my computer at the moment but there's the address calculation in setPixel method. There's the 7 - (x & 7) expression. Changing it to x & 7 may fix the problem. There's no other way to change the behaviour at the moment. I may add a define for it if needed.

Ok, so added bool modRev to globals and changed setPixel and getPixel, needed to flip y.

    uint8_t* p = _getBufferPtr(x,(modRev) ? 7 - y : y);
    if (!p)
        return;

    uint16_t b = (modRev) ? (x & 7) : 7 - (x & 7);          //bit
bartoszbielawski commented 5 years ago

Globals as in member or real global? The most important question - does it work as intended?

quakec commented 5 years ago

Globals as in member or real global? The most important question - does it work as intended?

Yes it works perfectly, by default it works the same way as it was out of the box and by passing true I get the results I need.

in the .cpp file...


LEDMatrixDriver::LEDMatrixDriver(uint8_t N, uint8_t ssPin, bool modRev_, uint8_t* frameBuffer_):
#ifdef USE_ADAFRUIT_GFX
    Adafruit_GFX(N*8, N),
#endif
    N(N),
    spiSettings(5000000, MSBFIRST, SPI_MODE0),
    modRev(modRev_),
    frameBuffer(frameBuffer_),
    selfAllocated(frameBuffer_ == nullptr),
    ssPin(ssPin)
{...

In the header file... altered definition for LEDMatrixDriver function...

    public:
        //with N segments and ssPin as SS,
        //an already allocated buffer can be provided as well
        LEDMatrixDriver(uint8_t N, uint8_t ssPin, bool modReverse = false, uint8_t* frameBuffer = nullptr);

modRev definition:

    private:
        uint8_t* _getBufferPtr(uint16_t x, uint16_t y) const;
        void _sendCommand(uint16_t command);
        void _displayRow(uint8_t row);

        const uint8_t N;
        SPISettings spiSettings;
        bool modRev;
        uint8_t* frameBuffer;
bartoszbielawski commented 5 years ago

I'm busy this weekend so maybe you could make a patch and a pull request? If not I will do it maybe Sunday :)

On Sat, 9 Feb 2019, 12:36 James <notifications@github.com wrote:

Globals as in member or real global? The most important question - does it work as intended? Yes it works perfectly, by default it works the same way as it was out of the box and by passing true I get the results I need.

in the .cpp file...

LEDMatrixDriver::LEDMatrixDriver(uint8_t N, uint8t ssPin, bool modRev, uint8t* frameBuffer):

ifdef USE_ADAFRUIT_GFX

Adafruit_GFX(N*8, N),

endif

N(N), spiSettings(5000000, MSBFIRST, SPIMODE0), modRev(modRev), frameBuffer(frameBuffer), selfAllocated(frameBuffer == nullptr), ssPin(ssPin) {...

In the header file... altered definition for LEDMatrixDriver function...

public: //with N segments and ssPin as SS, //an already allocated buffer can be provided as well LEDMatrixDriver(uint8_t N, uint8_t ssPin, bool modReverse = false, uint8_t* frameBuffer = nullptr);

modRev definition:

private: uint8_t* _getBufferPtr(uint16_t x, uint16_t y) const; void _sendCommand(uint16_t command); void _displayRow(uint8_t row);

  const uint8_t N;
  SPISettings spiSettings;
  **bool modRev;**
  uint8_t* frameBuffer;

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bartoszbielawski/LEDMatrixDriver/issues/21#issuecomment-462036957, or mute the thread https://github.com/notifications/unsubscribe-auth/AD2sEC6YuVVN14wb66bMx7hHT3dNnLYLks5vLrLNgaJpZM4axMGz .

quakec commented 5 years ago

although I feel inclined to have a separate function call to set this, since code which uses this driver before this change may pass values to frameBuffer.

bartoszbielawski commented 5 years ago

I wouldn't add it as a separate function. I'd just use another constructor. With C++11 you can call one c-tor from the other. No code broken, no extra functions.

quakec commented 5 years ago

Very well... I'll implement a delegating constructor :)

quakec commented 5 years ago

works beautifully... // delegating constructor : to set module reverse boolean option LEDMatrixDriver::LEDMatrixDriver(uint8_t N, uint8_t ssPin, bool modRev_, uint8_t* frameBuffer_): LEDMatrixDriver(N, ssPin, frameBuffer_) { modRev = modRev_; }

bartoszbielawski commented 5 years ago

Yup, that's what I wanted to have. Can you make a pull request with these changes?

quakec commented 5 years ago

Yup, that's what I wanted to have. Can you make a pull request with these changes?

Done :)

bartoszbielawski commented 5 years ago

Merged, thanks :)