sbma44 / cheerlights-arduino-python

Cheerlights implementation for GE Color Effects; logic in python, Arduino serves as interface only
4 stars 0 forks source link

Error (list Index of out range) #1

Open ocelma opened 11 years ago

ocelma commented 11 years ago

Hi,

I'm trying to run your library. I have G35 correctly connected to my Arduino board (I can run some Processing scripts and works fine). . V+ is on 5V pin . V- is on GND . Data is on PIN 4

Here are my settings: SERIAL_PORT = '/dev/ttyACM0' #'/dev/tty.usbmodem621' SERIAL_RATE = 115200 NUM_STRINGS = 1 LIGHTS_PER_STRING = 50 NUM_LIGHTS = NUM_STRINGS * LIGHTS_PER_STRING

When I run benchmark.py it with your code it gets stuck here: con.update_hue() : line 117 (on self.ser.readline())

If I comment this line (notice that line 102: # self.ser.readline() is also commented on your code), then it gives this Exception:

light_a = self.lights[string_offset + (i*2)]

--> IndexError: list index out of range

as it's accessing position 50 in self.lights (that has len: 50!).

Also, I don't get these lines:

self.ser.write(chr((light_a.r<<4) | light_a.g)) # Why not: | light_a.r instead?
self.ser.write(chr((light_a.b<<4) | light_b.r)) # Why not: | light_a.b instead?
self.ser.write(chr((light_b.g<<4) | light_b.b)) # etc.

So, when did this code work fine?

Thanks in advance,

sbma44 commented 11 years ago

Hi there -- I'm delighted that you're interested in the library! I used a modified version of it this New Year's with two strings, and I received a report last week from a friend who successfully adapted it as well with one string. So I can confirm that the code works.

Can you tell me what kind of system you're running on? I have to admit that serial port weirdness stopped me from running this successfully on both OS X and Windows. I wound up running Ubuntu, but even that gave me some weirdness after several days of use.

Also, are you sure that the sketch is loaded onto the Arduino properly? If it's not sending back characters (or the serial port is misbehaving), that would explain the blocking behavior.

As for the self.ser.write() lines, this is designed to pack 2 lights' data into 3 bytes. self.ser.write() sends a single byte. Each color channel has 4 bits of resolution. Those lines bit-shift the r, g, and b components by appropriate offsets, combining them into bytes (chars).

ocelma commented 11 years ago

Hi,

I'm running Ubuntu 12.10

color_effects_lights.pde sketch is indeed running onto Arduino (code verified, and the uploaded)

Using ipython, I can send:

serial.write('I")
serial.write(char(0xFF & 0xCC)
(...)

and it works fine

Thanks for the explanation on the self.update_hue() bits...!

sbma44 commented 11 years ago

I'm afraid I'm not sure how you could be getting to position 50, then, or why it's blocking. Have you tried adding some logging to identify how many lights it thinks there are? I admit that I haven't tested with one string, so it's quite possible that there's a bug.

ocelma commented 11 years ago

I've been doing some more tests...

I don't get why both update_intensity() and update_hue() have two inner loops. E.g:

for string_offset in (0, LIGHTS_PER_STRING):
    for i in range(0, LIGHTS_PER_STRING):
        l = self.lights[string_offset + i]
        (...)

This code always crashes, as it goes out of range!

Method init creates the list, with NUM_LIGHTS (in my case 50) length:

for i in range(0, NUM_LIGHTS):
    self.lights.append(Light().random_color())

And then update_intensity() does this:

for string_offset in (0, LIGHTS_PER_STRING):
    for i in range(0, LIGHTS_PER_STRING):
        l = self.lights[string_offset + i]

which, of course, crashes here:

l = self.lights[string_offset + i] # with string_offset = LIGHTS_PER_STRING and i >= 0 goes out of range!

Here's some code that works fine for 1 string (at least for me! :-)

def update_intensity(self):
    # send intensity
    self.ser.write('I')        
    #self.ser.readline() # unsure why this is necessary but it is

    #for string_offset in (0, LIGHTS_PER_STRING):
    for i in range(0, LIGHTS_PER_STRING):
        l = self.lights[i]
        l.safe()
        self.ser.write(chr(0xFF & l.intensity))
    # wait for ack -- do this twice        
    self.ser.readline()

Or better this way (not tested yet):

def update_intensity(self):
    # send intensity
    self.ser.write('I')        
    #self.ser.readline() # unsure why this is necessary but it is

    for string in (0, NUM_STRINGS):
        for i in range(0, LIGHTS_PER_STRING):
            l = self.lights[string + i] 
            l.safe()
            self.ser.write(chr(0xFF & l.intensity))
        # wait for ack -- do this twice        
        self.ser.readline()
sbma44 commented 11 years ago

I'm glad you've got it working! I typically don't use the intensity updating functions because it introduces flicker (a hardware limitation of the Color Effects lights).

The nested loops are there for verbosity -- keep in mind that the top loop isn't dealing with a range of (0, LIGHTS_PER_STRING), it's dealing with a tuple. So that loop is only iterated through twice. Each individual string is then run through.

I haven't dug into the code, but if memory serves this is necessary (rather than smoothly looping through all possible lights -- say, 0..99) because the GE Color Effects strings only have a 6 bit address space. Therefore, in the configuration I created, some separate signal has to be sent to the Arduino telling it which string to address.

Note that I never built out more than two strings, so additional code would be necessary to scale beyond that.

On Mon, Jan 14, 2013 at 5:58 PM, Oscar Celma notifications@github.comwrote:

I've been doing some more tests...

I don't get why both update_intensity() and update_hue() have two inner loops. E.g:

for string_offset in (0, LIGHTS_PER_STRING): for i in range(0, LIGHTS_PER_STRING): l = self.lightsstring_offset + i

This code always crashes, as it goes out of range!

Method init creates the list, with LIGHTS_PER_STRING (in my case 50) length:

for i in range(0, NUM_LIGHTS): self.lights.append(Light().random_color())

And then update_intensity() does this:

for string_offset in (0, LIGHTS_PER_STRING): for i in range(0, LIGHTS_PER_STRING): l = self.lights[string_offset + i]

which, of course, crashes here:

l = self.lights[string_offset + i] # with string_offset = LIGHTS_PER_STRING and i >= 0 goes out of range!

Here's some code that works fine for 1 string (at least for me! :-)

def update_intensity(self):

send intensity

self.ser.write('I')
#self.ser.readline() # unsure why this is necessary but it is
#for string_offset in (0, LIGHTS_PER_STRING):
for i in range(0, LIGHTS_PER_STRING):
    l = self.lights[i]
    l.safe()
    self.ser.write(chr(0xFF & l.intensity))
# wait for ack -- do this twice
self.ser.readline()

Or better this way (not tested yet):

def update_intensity(self):

send intensity

self.ser.write('I')
#self.ser.readline() # unsure why this is necessary but it is
for string in (0, NUM_STRINGS):
    for i in range(0, LIGHTS_PER_STRING):
        l = self.lights[string + i] # Assuming that self.lights is a list of NUM_STRINGS*LIGHTS_PER_STRING elements
        l.safe()
        self.ser.write(chr(0xFF & l.intensity))
    # wait for ack -- do this twice
    self.ser.readline()

— Reply to this email directly or view it on GitHubhttps://github.com/sbma44/cheerlights-arduino-python/issues/1#issuecomment-12244788.