Skip to content

Keep discovered sources for two discovery intervals - #79

Open
stewartadam wants to merge 1 commit into
mainfrom
claude/project-thread-njq8b9-5-discovery-source-timeout
Open

stewartadam wants to merge 1 commit into
mainfrom
claude/project-thread-njq8b9-5-discovery-source-timeout

Conversation

@stewartadam

@stewartadam stewartadam commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

UNIVERSE_DISCOVERY_SOURCE_TIMEOUT was set to E131_NETWORK_DATA_LOSS_TIMEOUT (2.5 s). Sources only send universe discovery packets every E131_UNIVERSE_DISCOVERY_INTERVAL (10 s, E1.31-2016 Appendix A). As a result, SacnReceiver::get_discovered_sources() dropped each source 2.5 s after its advert. It returned an empty list about 75% of the time, even though the source was still advertising.

The timeout is now twice the discovery interval (20 s), so a source is only treated as lost after it misses a whole advert. The standard doesn't define a receiver-side timeout. The old doc comment cited one, and it now explains the reasoning instead.

Tests

  • New test_discovered_source_kept_between_adverts: a source whose advert is 9.9 s old is still listed, and one older than the timeout is removed.
  • cargo test and cargo clippy pass, and clippy reports no new warnings. The ignored multicast discovery tests read discovered sources right after an advert, so the longer timeout doesn't affect them.

UNIVERSE_DISCOVERY_SOURCE_TIMEOUT was the 2.5 s network data loss
timeout, but sources only send universe discovery packets every
E131_UNIVERSE_DISCOVERY_INTERVAL (10 s). get_discovered_sources dropped
each source 2.5 s after its advert, so it was empty about three quarters
of the time even though the source was still advertising.

Use twice the discovery interval (20 s), so a source is only treated as
lost after missing an advert. E1.31 doesn't define a receiver timeout,
and the doc comment no longer cites one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzaQVQThQpmqhvLapk99Vs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants