PIO WS2812 example - bug in `pattern_fade()`

Open
#32 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
28/100
Issue type
Bug
Clarity
Needs clarification
Activity status
Stale
Tech stack
c
Domain
embedded-iot

Research direction

Start in pio/ws2812/ws2812_parallel.c at pattern_fade(), especially lines 96-97, and trace how slow_t and level are used afterward. Compare the current pixel output with the intended fade behavior and confirm the intended operator with project maintainers. Done means the example no longer alternates only between black and 0x010101 and the expected behavior is documented by a focused check.

Written by the indexing model from the issue text.

Description

This bug is found at the start of the function, pattern_fade():

  1. Line 96 sets slow_t to a value that depends on the second function parameter. This makes sense.
    https://github.com/raspberrypi/pico-examples/blob/33854562cd08398c4b48bb1ed7fa022c2177076d/pio/ws2812/ws2812_parallel.c#L96

  2. Line 97, however, sets slow_t to a fixed, constant value of 8 (variable level is defined at line 88, never modified).
    https://github.com/raspberrypi/pico-examples/blob/33854562cd08398c4b48bb1ed7fa022c2177076d/pio/ws2812/ws2812_parallel.c#L97

If I followed it correctly, the current code results in alternately setting all pixels to RGB 0x000000 (black), or setting all pixels to RGB 0x010101. Clearly not the intended behavior.

Most likely, line 97 was intended to be an operator that modified the existing value, such as any of:

    slow_t += level; 
    slow_t -= level; 
    slow_t *= level; 
    slow_t /= level; 
    slow_t <<= level; 
    slow_t >>= level;

Unfortunately, I am note 100% sure what the intended result was.

My best guess? It's an attempt to add half the potential error, so integer division / masking later makes calculating the error amount easier.

    slow_t += level; // increase provided value by half of (1 << shift)?

@kilograham ... any ideas who would know the intended results here, and might confirm the intended behavior here?

Thanks!

Dominant language
C
Stars
3.9k
Forks
1k
Avg merge
1d 16h
Merged PRs (30d)
1

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from raspberrypi/pico-examples

All issues in raspberrypi/pico-examples

Similar issues

More C issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.