Skip to content

Lights and FAST EXP code cleanup#1943

Open
bosh wants to merge 7 commits intomissionpinball:devfrom
bosh:lights_and_exp_cleanup
Open

Lights and FAST EXP code cleanup#1943
bosh wants to merge 7 commits intomissionpinball:devfrom
bosh:lights_and_exp_cleanup

Conversation

@bosh
Copy link
Collaborator

@bosh bosh commented Jan 4, 2026

Nonbreaking refactors and cleanup in the lighting code in prep for FAST EXP ER support

# for one channel default to a white channel
color_channels = "w"
elif len(channel_list) == 3:
# TODO this seems like a bug waiting to happen -- what if the channel list ISNT R G B ordered?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if the channel list isn't RGB ordered then the user should specify a type to clarify what the channels are. It's a fair assumption that if the user doesn't specify, we have some type of fallback.

Alternatively, we could make type required with a default value of "rgb"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case in particular only happens if you dont have a config[channels] or a config[type], so I guess really the user is letting the platform defaults do their thing. Since channel_list is always from a simple number to channel set generation via parse_light_number_to_channels, it's not really a userland bug that could happen here, it's a platform not defining a set of three channels as exactly r g then b.

So maybe the comment is too harsh, and all we need is documentation on the parse_light_number_to_channels interface that it must return an rgb-ordered channel list

@bosh bosh force-pushed the lights_and_exp_cleanup branch from 649456c to 98efd4f Compare January 20, 2026 03:25
@bosh bosh marked this pull request as ready for review January 28, 2026 07:52
@bosh bosh force-pushed the lights_and_exp_cleanup branch from 98efd4f to 1be2006 Compare February 8, 2026 07:13
@bosh bosh added the ready label Feb 11, 2026
@bosh bosh force-pushed the lights_and_exp_cleanup branch from 1be2006 to c21e745 Compare February 14, 2026 05:58
bosh added 7 commits February 16, 2026 01:54
…h > 1 carry value

though this should never happen in practice since we only call loop range(3) times,
so +0, +1, and +2. Even a 4-channel light could only carry once, so
really this should only matter in the 5+ channel case (or perhaps if you
can start with an offset of +3, which really should be a carry+1 with 0 offset
for readability. The config params are generally pulled out inside the initialize
in these, rather than from the self config reference within the function -- this
is intentional to try and keep config reading inside initialize itself
to better orient the seams with the data structures and relevancy
@bosh bosh force-pushed the lights_and_exp_cleanup branch from c21e745 to dd67090 Compare February 16, 2026 09:54
@sonarqubecloud
Copy link

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants