Skip to content

Use correct start index for Art-Net in proxy mode#3863

Open
askask wants to merge 1 commit intowled:mainfrom
askask:0_15
Open

Use correct start index for Art-Net in proxy mode#3863
askask wants to merge 1 commit intowled:mainfrom
askask:0_15

Conversation

@askask
Copy link
Contributor

@askask askask commented Mar 29, 2024

Art-Net data doesn't contain a start byte. This is currently only taken into account when using regular LED output.
With this commit it also works when using DMX proxy mode.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed DMX data alignment across Art-Net and E1.31 lighting protocols for proper channel synchronization and correct data handling when using multiple protocol paths.

✏️ Tip: You can customize this high-level summary in your review settings.

@softhack007
Copy link
Member

@troyhacks, @netmindz: If I remember correctly, you are regularly working with Art-Net? Would like to get your thoughts on this PR.

@troyhacks
Copy link
Contributor

I think I understand it - it's just aligning incoming Art-Net/E1.31 data so channels from an Art-Net packet hits DMX output on the correct channel.

I haven't used this feature, but I think it's safe to merge. Good catch, @askask!

@askask askask marked this pull request as draft April 8, 2024 21:35
@askask askask marked this pull request as ready for review April 9, 2024 01:33
@github-actions
Copy link

github-actions bot commented Aug 7, 2024

Hey! This pull request has been open for quite some time without any new comments now. It will be closed automatically in a week if no further activity occurs.
Thank you for contributing to WLED! ❤️

@github-actions github-actions bot added the stale This issue will be closed soon because of prolonged inactivity label Aug 7, 2024
@blazoncek blazoncek added keep This issue will never become stale/closed automatically and removed stale This issue will be closed soon because of prolonged inactivity labels Aug 7, 2024
@netmindz
Copy link
Member

netmindz commented Aug 8, 2024

I'm not sure WLED is really appropriate for trying to be a an ArtNet/sACN to DMX (wired) convertor, but if has that feature it should certainly map the right values to the right channel.

If we are looking for confirmation of fix before merge with actual hardware I can probably rustle something up.

For future reference I would always recommend using a dedicated esp32 if you want such bridge from wireless to wired DMX, lighting desk send a universe at up to 44 Hz which WLED can't provide so you will lose frames and moving heads might behave erratically when getting a slow and inconsistent update

@softhack007 softhack007 added this to the 0.15.1 candidate milestone Sep 29, 2024
@netmindz netmindz changed the base branch from 0_15 to main December 20, 2024 14:06
@netmindz netmindz modified the milestones: 0.15.1 candidate, 0.15.2 Feb 22, 2025
@netmindz netmindz modified the milestones: 0.15.2, 0.15.3 Nov 17, 2025
@softhack007
Copy link
Member

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 28, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 28, 2026

Walkthrough

Introduces an artnetOffset variable to unify DMX data indexing between Art-Net and E1.31 protocols in e131.cpp. The DMX write logic now applies this offset during data access, and dataOffset adjustment is consolidated into a single calculation path instead of conditional branching.

Changes

Cohort / File(s) Summary
DMX Protocol Offset Handling
wled00/e131.cpp
Introduces artnetOffset variable to normalize indexing differences between Art-Net (offset −1) and E1.31 (offset 0). Modifies handleE131Packet to apply offset during DMX data read operations and consolidates dataOffset adjustment logic, removing separate Art-Net conditional decrement.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: correcting Art-Net start index handling in proxy mode to align incoming data with DMX output channels.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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

Labels

Awaiting testing bug keep This issue will never become stale/closed automatically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants