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.

Sign up for GitHub

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

Jump to bottom

Add strip pulsing feature to RainAudioEffect class #591

Merged
merged 1 commit into from
Jan 18, 2024
Merged

Add strip pulsing feature to RainAudioEffect class #591

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 42 additions & 13 deletions ledfx/effects/rain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ class RainAudioEffect(AudioReactiveEffect):
description="color for low sounds, ie beats",
default="white",
): validate_color,
vol.Optional(
"pulse_strip",
description="Pulse the entire strip to the beat",
default="Off",
): vol.In(["Off", "Lows", "Mids", "Highs"]),
Comment on lines +30 to +34
Copy link
Member Author

Choose a reason for hiding this comment

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

This is one of our oldest effects - and I must be honest, the lack of a config_updated function in here confuses me a little but everything is working as expected.

I wonder if assigning to self rather than grabbing the config out of ledfx is faster/more performant, but it'd be interesting to see the work/effort/time/performance trade off for using the config values vs assigning to self for every config value that we add.

Copy link
Contributor

Choose a reason for hiding this comment

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

The difference between variable assignment and key resolution in the config is probably difficult to measure, for non iterative behaviors, but when you get into loops, I expect it to add up quickly.

vol.Optional(
"mids_color",
description="color for mid sounds, ie vocals",
Expand Down Expand Up @@ -63,6 +68,7 @@ class RainAudioEffect(AudioReactiveEffect):
def on_activate(self, pixel_count):
self.drop_frames = np.zeros(self.pixel_count, dtype=int)
self.drop_colors = np.zeros((3, self.pixel_count))
self.pulse_pixels = np.zeros((self.pixel_count, 3))

def config_updated(self, config):
self.drop_animation = load_droplet(config["raindrop_animation"])
Expand Down Expand Up @@ -103,6 +109,8 @@ def render(self):
drop_indices = np.flatnonzero(self.drop_frames)
# TODO vectorize this to remove for loop
for index in drop_indices:
if self.drop_frames[index] >= len(self.drop_animation):
continue
colored_frame = [
self.drop_animation[self.drop_frames[index]]
* self.drop_colors[color, index]
Expand All @@ -112,12 +120,24 @@ def render(self):
:, index : index + self.frame_width
] += colored_frame

np.clip(overlaid_frames, 0, 255, out=overlaid_frames)
self.pixels = overlaid_frames[
:,
self.frame_side_lengths : self.frame_side_lengths
+ self.pixel_count,
].T
self.pixels += self.pulse_pixels
# Decay the pulse pixels
self.pulse_pixels = (self.pulse_pixels * 9) // 10
shauneccles marked this conversation as resolved.
Show resolved Hide resolved

def strip_pulse(self, color):
"""
Set the pulse pixels to a color
This color decays over time in render()

Args:
color: The color to pulse the strip
"""
self.pulse_pixels = np.array([color])
Comment on lines +132 to +140
Copy link
Contributor

Choose a reason for hiding this comment

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

The strip_pulse method sets the entire pulse_pixels array to the same color, which may not be the intended behavior if pulse_pixels is supposed to be a 2D array with the same color for each pixel.

- self.pulse_pixels = np.array([color])
+ self.pulse_pixels = np.tile(color, (self.pixel_count, 1))
Committable suggestion

❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
def strip_pulse(self, color):
"""
Set the pulse pixels to a color
This color decays over time in render()
Args:
color: The color to pulse the strip
"""
self.pulse_pixels = np.array([color])
def strip_pulse(self, color):
"""
Set the pulse pixels to a color
This color decays over time in render()
Args:
color: The color to pulse the strip
"""
self.pulse_pixels = np.tile(color, (self.pixel_count, 1))


def audio_data_updated(self, data):
# Calculate the low, mids, and high indexes scaling based on the pixel
Expand All @@ -132,25 +152,34 @@ def audio_data_updated(self, data):
intensities[0] - self.filtered_intensities[0]
> self._config["lows_sensitivity"]
):
self.new_drop(
randint(0, self.pixel_count - 1),
parse_color(self._config["lows_color"]),
)
if self._config["pulse_strip"] == "Lows":
self.strip_pulse(parse_color(self._config["lows_color"]))
else:
self.new_drop(
randint(0, self.pixel_count - 1),
parse_color(self._config["lows_color"]),
)
if (
intensities[1] - self.filtered_intensities[1]
> self._config["mids_sensitivity"]
):
self.new_drop(
randint(0, self.pixel_count - 1),
parse_color(self._config["mids_color"]),
)
if self._config["pulse_strip"] == "Mids":
self.strip_pulse(parse_color(self._config["mids_color"]))
else:
self.new_drop(
randint(0, self.pixel_count - 1),
parse_color(self._config["mids_color"]),
)
if (
intensities[2] - self.filtered_intensities[2]
> self._config["high_sensitivity"]
):
self.new_drop(
randint(0, self.pixel_count - 1),
parse_color(self._config["high_color"]),
)
if self._config["pulse_strip"] == "Highs":
self.strip_pulse(parse_color(self._config["high_color"]))
else:
self.new_drop(
randint(0, self.pixel_count - 1),
parse_color(self._config["high_color"]),
)

self.filtered_intensities = self.intensity_filter.update(intensities)