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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

omar711
Copy link

@omar711 omar711 commented Oct 29, 2024

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 omar711 marked this pull request as ready for review October 29, 2024 12:02
@omar711
Copy link
Author

omar711 commented Oct 29, 2024

To add my hardware:

  • Rasperrypi 4, model B
  • SenseHat version 2
  • SenseHat library is the latest, 2.6.0

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

Successfully merging this pull request may close these issues.

1 participant