astro-pi / python-sense-hat

Source code for Sense HAT Python library
https://sense-hat.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
510 stars 255 forks source link

Scale by ceil(max_raw / 256) to prevent out of range colour values #146

Open omar711 opened 1 month ago

omar711 commented 1 month ago

I was able to get out of range colour values from sense.color and noticed that the scaling of colour values allows rbg values up to 257.

A simple repro script is:

from sense_hat import SenseHat
from time import sleep

sense = SenseHat()
sense.set_rotation(270, False)

sense.color.gain = 64 
sense.color.integration_cycles = 64 

while True:
    rgb = sense.color
    c = (rgb.red, rgb.green, rgb.blue)
    print(c, sense.color.color_raw)

    sense.clear(c)

    sleep(1)

When you run this and shine a torch at the sensor, you'll see:

$ python out_of_range_repro.py 
(1, 1, 1) (300, 301, 277, 693)
(71, 71, 65) (18181, 18240, 16823, 42086)
(70, 71, 65) (18094, 18106, 16740, 41803)
(68, 68, 63) (17384, 17407, 16136, 40250)
(73, 82, 69) (18711, 21028, 17845, 48307)
(257, 257, 257) (65535, 65535, 65535, 65535)
Traceback (most recent call last):
  File "/home/omar/missionzero/out_of_range_repro.py", line 15, in <module>
    sense.clear(c)
  File "/usr/lib/python3/dist-packages/sense_hat/sense_hat.py", line 443, in clear
    self.set_pixels([colour] * 64)
  File "/usr/lib/python3/dist-packages/sense_hat/sense_hat.py", line 318, in set_pixels
    raise ValueError('Pixel at index %d is invalid. Pixel elements must be between 0 and 255' % index)
ValueError: Pixel at index 0 is invalid. Pixel elements must be between 0 and 255

I believe it's this change where max_value changes from 65536 to 65535, that allows the problem to occur in the _scaling() method.

I've adjusted the scaling method to use ceil(max_raw / 256), which for max inputs of 65535 will give a scaling factor of 256 as it would have done prior to #131 . I used the method ceildiv to avoid extra imports, plus it seems to be favoured by stackoverflow.

Running the repro again with this change:

$ python out_of_range_repro.py 
(1, 1, 0) (270, 273, 254, 629)
(71, 71, 66) (18176, 18321, 16958, 42086)
(73, 73, 68) (18937, 18869, 17424, 43437)
(120, 132, 105) (30785, 33801, 27111, 57861)
(255, 255, 255) (65535, 65535, 65535, 65535)
(255, 255, 255) (65535, 65535, 65535, 65535)
(255, 255, 255) (65535, 65535, 65535, 65535)
(255, 255, 255) (65535, 65535, 65535, 65535)
(255, 255, 255) (65535, 65535, 65535, 65535)
(255, 255, 255) (65535, 65535, 65535, 65535)
omar711 commented 1 month ago

To add my hardware: