New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SPI Plugin - added APA102 #722

Merged
merged 49 commits into from Jun 3, 2015
Commits
Jump to file or symbol
Failed to load files and symbols.
+12 −10
Diff settings

Always

Just for now

Viewing a subset of changes. View all

fixed some bugs

  • Loading branch information...
s-light committed May 5, 2015
commit fbb0de7ec4d818710c46fd789251e0e7f7d78bcb
View
@@ -509,8 +509,8 @@ void SPIOutput::IndividualAPA102Control(const DmxBuffer &buffer) {
// LEDFrame: 1Byte FF ; 3Byte color info (Blue, Green, Red)
// EndFrame: (n/2)bits; n = pixel_count
// i don't know how this works.. but it works ;-)
const uint8_t latch_bytes = 3 * APA102_SPI_BYTES_PER_PIXEL;
// latch_bytes = EndFrame ??

This comment has been minimized.

@peternewman

peternewman May 10, 2015

Member

Can you remove this comment, or at least the question marks please.

@peternewman

peternewman May 10, 2015

Member

Can you remove this comment, or at least the question marks please.

const uint8_t latch_bytes = CalculateAPA102LatchBytes(m_pixel_count);
const unsigned int first_slot = m_start_address - 1; // 0 offset
// only do something if minimum 1pixel can be updated..

This comment has been minimized.

@peternewman

peternewman May 18, 2015

Member

Nit: perhaps rewrite this to "Only do something if at least one pixel can be updated..."

@peternewman

peternewman May 18, 2015

Member

Nit: perhaps rewrite this to "Only do something if at least one pixel can be updated..."

@@ -568,7 +568,7 @@ void SPIOutput::CombinedAPA102Control(const DmxBuffer &buffer) {
}
// get data for entire string length
const unsigned int output_length = (m_pixel_count + 1) *
const unsigned int output_length = (m_pixel_count + 1) *
APA102_SPI_BYTES_PER_PIXEL;

This comment has been minimized.

@peternewman

peternewman May 28, 2015

Member

Can you line this up with the start of the (, or four after const

@peternewman

peternewman May 28, 2015

Member

Can you line this up with the start of the (, or four after const

uint8_t *output = m_backend->Checkout(m_output_number, output_length,
latch_bytes);

This comment has been minimized.

@nomis52

nomis52 May 7, 2015

Member

I'd call CalculateAPA102LatchBytes(m_pixel_count) here and remove the temporary. Same in the method above as well.

@nomis52

nomis52 May 7, 2015

Member

I'd call CalculateAPA102LatchBytes(m_pixel_count) here and remove the temporary. Same in the method above as well.

@@ -597,10 +597,13 @@ void SPIOutput::CombinedAPA102Control(const DmxBuffer &buffer) {
* Calculate Latch Bytes for APA102:
* minimal use half the pixel count bits

This comment has been minimized.

@peternewman

peternewman May 18, 2015

Member

Nit: "Use at least half the pixel count bits"

@peternewman

peternewman May 18, 2015

Member

Nit: "Use at least half the pixel count bits"

* round up to next full byte count.
* datasheet says endframe should consist of 4 bytes - but thats only valid for
* up to 64 pixels/leds.
* datasheet says endframe should consist of 4 bytes -
* but thats only valid for up to 64 pixels/leds.
*
* the function is valid up to 4080 pixels. (255*8*2)
* ( otherwise the return type must be changed to uint16_t)
*/
void SPIOutput::CalculateAPA102LatchBytes(unsigned int m_pixel_count) {
uint8_t SPIOutput::CalculateAPA102LatchBytes(unsigned int m_pixel_count) {

This comment has been minimized.

@nomis52

nomis52 May 7, 2015

Member

m_pixel_count shadows the member variable with the same name. Call this pixel_count and make the method static.

@nomis52

nomis52 May 7, 2015

Member

m_pixel_count shadows the member variable with the same name. Call this pixel_count and make the method static.

const uint8_t latch_bits = m_pixel_count / 2;
const uint8_t latch_bytes = (latch_bits / 8) + (latch_bits % 8 ? 1: 0);

This comment has been minimized.

@nomis52

nomis52 May 7, 2015

Member

You can just return on this line and save the intermediate variable.

@nomis52

nomis52 May 7, 2015

Member

You can just return on this line and save the intermediate variable.

This comment has been minimized.

@peternewman

peternewman May 10, 2015

Member

Ceil again. Also once we've converted it, we can combine them both into one function:
return ceil(ceil(m_pixel_count/2)/8)

@peternewman

peternewman May 10, 2015

Member

Ceil again. Also once we've converted it, we can combine them both into one function:
return ceil(ceil(m_pixel_count/2)/8)

return latch_bytes;
@@ -469,7 +469,7 @@ void SPIOutputTest::testIndividualAPA102Control() {
data = backend.GetData(0, &length);
const uint8_t EXPECTED1[] = { 0, 0, 0, 0,
0xff, 0, 0x80, 0xff,
0xff, 0x1e, 0x14, 0x0a,
0xff, 0x1e, 0x14, 0x0a,
0, 0, 0, 0, 0, 0, 0, 0};
OLA_ASSERT_DATA_EQUALS(EXPECTED1, arraysize(EXPECTED1), data, length);
OLA_ASSERT_EQ(2u, backend.Writes(0));
@@ -480,7 +480,7 @@ void SPIOutputTest::testIndividualAPA102Control() {
data = backend.GetData(0, &length);
const uint8_t EXPECTED2[] = { 0, 0, 0, 0,
0xff, 0x4e, 0x38, 0x22,
0xff, 0x1e, 0x14, 0x0a,
0xff, 0x1e, 0x14, 0x0a,
0, 0, 0, 0, 0, 0, 0, 0};
OLA_ASSERT_DATA_EQUALS(EXPECTED2, arraysize(EXPECTED2), data, length);
OLA_ASSERT_EQ(3u, backend.Writes(0));
@@ -504,7 +504,7 @@ void SPIOutputTest::testIndividualAPA102Control() {
data = backend.GetData(0, &length);
const uint8_t EXPECTED4[] = { 0, 0, 0, 0,
0xff, 0x05, 0x04, 0x03,
0xff, 0x08, 0x07, 0x06,
0xff, 0x08, 0x07, 0x06,
0, 0, 0, 0, 0, 0, 0, 0};
OLA_ASSERT_DATA_EQUALS(EXPECTED4, arraysize(EXPECTED4), data, length);
OLA_ASSERT_EQ(4u, backend.Writes(0));
@@ -604,5 +604,4 @@ void SPIOutputTest::testCombinedAPA102Control() {
OLA_ASSERT_EQ(reinterpret_cast<const uint8_t*>(NULL),
backend.GetData(1, &length));
OLA_ASSERT_EQ(0u, backend.Writes(1));
}
ProTip! Use n and p to navigate between commits in a pull request.