Skip to content

Install udev rules only if building DVStar module#4254

Open
dawkagaming wants to merge 2 commits into
aethersdr:mainfrom
dawkagaming:conditional-udev-install
Open

Install udev rules only if building DVStar module#4254
dawkagaming wants to merge 2 commits into
aethersdr:mainfrom
dawkagaming:conditional-udev-install

Conversation

@dawkagaming

Copy link
Copy Markdown
Contributor

Hello,

I have noticed while packaging the newest version for Debian, that even when you don't build the DVStar subsystem the udev rules are placed there, so I made a PR to fix this.

Also, the metainfo file should contain section about compatible hardware IDs, so it can recommend users who have the particular hardware device the AetherSDR software 😄

Thanks,
Dawid Kulas
SP9SKA

@dawkagaming
dawkagaming requested a review from a team as a code owner July 15, 2026 10:49

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks Dawid — gating the ThumbDV udev rules on the DVStar module is the right call; there's no reason to drop them on a build that has no aether-dv-waveform target. Moving the block inside if(TARGET aether-dv-waveform) and switching CMAKE_SYSTEM_NAME STREQUAL "Linux" to LINUX are both fine (min CMake is 3.25, and LINUX is already used at line 1121).

One thing I'd like fixed before merge: the DESTINATION also changed from lib/udev/rules.d to ${CMAKE_INSTALL_LIBDIR}/udev/rules.d, which will actually break the rules install on the distros this PR targets — see the inline note.

Would like fixed before merge

  • udev only scans /usr/lib/udev/rules.d; ${CMAKE_INSTALL_LIBDIR} resolves to a path udev never reads on multilib/multiarch distros. Keep the hardcoded lib/udev/rules.d.

Non-blocking note

  • The metainfo hardware-IDs idea from your description isn't part of this diff — happy to look at it as a separate PR.

🤖 aethersdr-agent · cost: $4.0154 · model: claude-opus-4-8

Comment thread CMakeLists.txt Outdated
Co-authored-by: aethersdr-agent[bot] <273844287+aethersdr-agent[bot]@users.noreply.github.com>
Signed-off-by: Dawid Kulas <147207983+dawkagaming@users.noreply.github.com>
@dawkagaming

Copy link
Copy Markdown
Contributor Author

Okay, sorry for that. I have accepted the suggested commit 😄

@jensenpat

Copy link
Copy Markdown
Collaborator

@dawkagaming Can you file an issue about the compatible hardware IDs so we can track and fix the documentation? Thank you

@dawkagaming

Copy link
Copy Markdown
Contributor Author

@jensenpat From this discussion: https://nw-digital-radio.groups.io/g/ambe/topic/ambe_server_install_no/102291698 - it looks like the ID is 0403:6015

@jensenpat
jensenpat marked this pull request as draft July 17, 2026 16:19
@jensenpat

Copy link
Copy Markdown
Collaborator

Holding for next week's release.

@jensenpat
jensenpat marked this pull request as ready for review July 19, 2026 03:24
@ten9876

ten9876 commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

First-pass triage review (not exhaustive — surfacing blockers to keep this moving):

Blockers
None spotted in a first pass. The udev-rules install() is correctly relocated inside the if(TARGET aether-dv-waveform) guard (CMakeLists.txt:3587), so the rules now install only when the DVStar waveform is actually built — which matches the title. The switch from CMAKE_SYSTEM_NAME STREQUAL "Linux" to if(LINUX) is safe here: the project requires cmake_minimum_required(VERSION 3.25) (CMakeLists.txt:1) and LINUX is defined as of 3.25, plus the codebase already uses it (e.g. line 1121), so this is consistent with house style.

Secondary notes

  • DESTINATION lib/udev/rules.d is a hardcoded path rather than derived from CMAKE_INSTALL_* — but that's pre-existing (unchanged by this PR), and udev rules conventionally live under /lib/udev, so no action needed here.
  • The metainfo hardware-ID suggestion in your description isn't part of this diff; worth a separate PR/issue if you'd like to pursue it.

Looks good on a first pass. Thanks!

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.

3 participants