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

adding alternative TLC59711Multi driver implementation #5

Open
wants to merge 43 commits into
base: master
from

Conversation

@s-light
Copy link

s-light commented Jan 6, 2019

adds TLC59711Multi
→ this new Implementation is multi-chip aware.
the API is compatible with FancyLED (similar to the neopixel/dotstar).

It has a bunch of different options to set pixel values - these have different speed results.
i have add a examples/tlc59711_multi_dev.py file with some speed comparisons.
the direct list setter pixels[pixel_index] = color supports float and int as tuple / list with auto conversion and extended error raising (reports what goes wrong and why)
this way its a little slower than the unprotected alternatives:
the fastest one is pixels.set_pixel_16bit_value(pixel_index, value_r, value_g, value_b)

this way the user can use the method that fits his application best..

all tests were done with 128 Pixel (= 32 chips)

pleas let me know what you think about this.

s-light added 30 commits Nov 16, 2018
by inlining functions :-(
faster: 0.16ms per pixel
by only checking value range once
more inlinening..
to if with helpfull messages
s-light added 11 commits Jan 5, 2019
this needs about 5ms more @128pixel
…Python_TLC59711 into multi
@s-light

This comment has been minimized.

Copy link
Author

s-light commented Jan 6, 2019

hm the build fails with C: 1, 0: Too many lines in module (1136/1000) (too-many-lines)
but i don't know what to delete...
i know this is a big driver with really a bunch of alternate setters...
and heavily commented...

for better name compatibility to TLC5957
@kattni kattni requested a review from adafruit/circuitpythonlibrarians Feb 20, 2019
Copy link

dhalbert left a comment

Code could be made a lot more compact in various ways. Consider a thorough pass-through to clean up.

# access is by design for the internal class.
#pylint: disable=protected-access
# context for using internal decorate classes below.
# In these cases protectected access is by design for the internal class.

This comment has been minimized.

Copy link
@dhalbert
# helper
##########################################

CHIP_BUFFER_BYTE_COUNT = 28

This comment has been minimized.

Copy link
@dhalbert

dhalbert Jul 15, 2019

Do these need to be constants visible to the end user? If not, use from micropython import const and then make these const(...), with names that begin with underscores. That will save space on importing.

BUFFER_BYTES_PER_PIXEL = BUFFER_BYTES_PER_COLOR * COLORS_PER_PIXEL

@staticmethod
def set_bit_with_mask(v, mask, x):

This comment has been minimized.

Copy link
@dhalbert

dhalbert Jul 15, 2019

I don't see this is used anywhere.

return v

@staticmethod
def set_bit(v, index, x):

This comment has been minimized.

Copy link
@dhalbert

dhalbert Jul 15, 2019

I don't see this is used anywhere.

value=0
):
"""Set chip header bits in buffer."""
# print(

This comment has been minimized.

Copy link
@dhalbert

dhalbert Jul 15, 2019

Remove debugging prints

return Ioclmax

@staticmethod
def calculate_Riref(

This comment has been minimized.

Copy link
@dhalbert

dhalbert Jul 15, 2019

Put args on one line.

"".format(Ioclmax)
)
if not 0.0 <= IoutR <= Ioclmax:
raise ValueError(

This comment has been minimized.

Copy link
@dhalbert

dhalbert Jul 15, 2019

Put on one or two lines, and below.


##########################################

def _debug_print_buffer(self):

This comment has been minimized.

Copy link
@dhalbert

dhalbert Jul 15, 2019

Can remove these debug print helpers now.


##########################################

def _get_32bit_value_from_buffer(self, buffer_start):

This comment has been minimized.

Copy link
@dhalbert

dhalbert Jul 15, 2019

Use struct instead.

This comment has been minimized.

Copy link
@dhalbert

dhalbert Jul 15, 2019

And same for routines below.


@staticmethod
def _check_and_convert(value):
# check if we have float values

This comment has been minimized.

Copy link
@dhalbert

dhalbert Jul 15, 2019

Could rewrite this as a loop.

@s-light

This comment has been minimized.

Copy link
Author

s-light commented Jul 15, 2019

thanks for your review!
i will do the changes and cleanup - but i think i will need some time - to much other things currently going on..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.