Skip to content
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

PixelBuf: TM1814 needs to dynamically modify the header bytes to set brightness #9729

Closed
jepler opened this issue Oct 18, 2024 · 2 comments · Fixed by #9730
Closed

PixelBuf: TM1814 needs to dynamically modify the header bytes to set brightness #9729

jepler opened this issue Oct 18, 2024 · 2 comments · Fixed by #9730

Comments

@jepler
Copy link
Member

jepler commented Oct 18, 2024

TM1814 LED strips use an overall strip brightness value that is transmitted as the first 64 bits of data.

To allow the brightness to be dynamically updated, these 8 bytes of the header need to be modifiable.

However, the "transmit buffer" is a read-only object and is only intended to be available inside the _transmit method, so there appears to be no way to update the header (or, for that matter, trailer) values.

Possible approaches:

  1. A new pair of methods "update_{header,trailer}" accept a buffer of equal length to the original header/trailer and update in place
  2. A pair of new properties header and trailer are added, which are writable buffers
  3. A new property buffer is added, which is a writable buffer
  4. (the above but as methods)
  5. A new constructor variant accepts a writable buffer object (this style of constructor would be incompatible with the one that takes header/trailer); the caller would construct a larger buffer with room for the header & trailer and pass in a sliced memoryview of it to the PixelBuf

For now, the brightness of TM1824 strips will just not be settable after construction time.

@jepler
Copy link
Member Author

jepler commented Oct 18, 2024

@tannewt I have a vague recollection that you had a good and strong motivation for NOT letting the caller pass in the buffer, and preferred to add header/trailer arguments instead. Is my memory right? Can you reconstruct your position on this?

@tannewt
Copy link
Member

tannewt commented Oct 18, 2024

@jepler I think it was to save memory by not allocating the post-brightness buffer until a non-1.0 brightness was set.

I think an option 6 where transmit is given a bytearray instead of a bytes object may be best. That way the transmit function can mutate it (to set brightness) before transmitting it. The buffer still stays "internal" then too. The subclass will want to override the brightness property so that the internal post-brightness buffer is never allocated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants