Skip to content

Refactor wled-tools discover_devices for deduplication and clarity#5493

Merged
netmindz merged 2 commits intowled:mainfrom
LordMike:patch-2
Apr 12, 2026
Merged

Refactor wled-tools discover_devices for deduplication and clarity#5493
netmindz merged 2 commits intowled:mainfrom
LordMike:patch-2

Conversation

@LordMike
Copy link
Copy Markdown
Contributor

@LordMike LordMike commented Apr 10, 2026

I've discovered that when we have multiple network interfaces, possible on different VLANs, which see the same mDNS entries, avahi will emit multiple entries (since it also keys on interface).

This change will make awk deduplicate, by only emitting unique name/ip/port pairs.

Summary by CodeRabbit

  • Bug Fixes
    • Device discovery now removes duplicate entries so each discovered device appears only once in scan results.
    • Discovery output remains compatible with existing parsing — format and emitted triplets are preserved to avoid breaking downstream consumers.

Refactor discover_devices function to deduplicate device entries and improve readability.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1e5ddcc6-28c5-4b74-a702-945d82c624e2

📥 Commits

Reviewing files that changed from the base of the PR and between a51aec6 and 97dacb6.

📒 Files selected for processing (1)
  • tools/wled-tools

Walkthrough

discover_devices() in tools/wled-tools now deduplicates mDNS discovery output by emitting only the first occurrence of each hostname\x1Faddress\x1Fport tuple; the output format and downstream parsing contract are unchanged.

Changes

Cohort / File(s) Summary
mDNS Device Discovery Deduplication
tools/wled-tools
discover_devices() updated to deduplicate avahi-browse results via an awk seen-map keyed by hostname\x1Faddress\x1Fport; only the first matching entry is printed, preserving the existing hostname\x1Faddress\x1Fport output format.

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 describes the main change: refactoring discover_devices for deduplication and clarity, which matches the PR's core objective of deduplicating mDNS entries.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@LordMike
Copy link
Copy Markdown
Contributor Author

I am unsure why the original wled-tools has so many spaces after each line.. it makes the diff weird.

@LordMike LordMike changed the title Refactor discover_devices for deduplication and clarity Refactor wled-tools discover_devices for deduplication and clarity Apr 10, 2026
@softhack007
Copy link
Copy Markdown
Member

softhack007 commented Apr 10, 2026

I am unsure why the original wled-tools has so many spaces after each line.. it makes the diff weird.

@LordMike yeah probably the reason is "it doesn't matter". Normally we recommend "don't try to optimize out trailing whitespaces". As you say, it makes the diff hard to read and it makes cherry-picking between branches a nightmare.

If you have any VSCode add-on that "optimizes" out trailing spaces, just remove them. Also don't use clang-format on the complete file before committing a PR.

@LordMike
Copy link
Copy Markdown
Contributor Author

I cleaned it up

@softhack007
Copy link
Copy Markdown
Member

I cleaned it up

Thanks, that's much clearer 👍

@softhack007

This comment was marked as outdated.

@coderabbitai

This comment was marked as outdated.

@softhack007 softhack007 added the optimization re-working an existing feature to be faster, or use less memory label Apr 10, 2026
Copy link
Copy Markdown
Member

@softhack007 softhack007 left a comment

Choose a reason for hiding this comment

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

looks fine for me.
I can't test it, but the only functional difference is print key => if (!seen[key]++) print key. everything else is just styling.

@netmindz netmindz merged commit 36ebcb5 into wled:main Apr 12, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

optimization re-working an existing feature to be faster, or use less memory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants