adds option ENABLE_AVAHI

[grutz]

@guruofquality Is there a blocker for merging this change?

[guruofquality]

@guruofquality Is there a blocker for merging this change?

I think its fine. I guess Im not sure of the different between ENABLE_AVAHI and automatically searching for package with AVAHI_FOUND. Is the idea to better support disabling use of avahi by silencing the warning printed when avahi is not found by passing -DENABLE_AVAHI=OFF

[zuckschwerdt]

A rule for good cmake options that gets handed around is: do exactly as selected with errors instead of fallbacks.

I.e. -DENABLE_AVAHI=OFF should never include Avahi; -DENABLE_AVAHI=ON must include Avahi or error (not the case currently); and the somewhat accepted middle ground would be a third -DENABLE_AVAHI=AUTO choice.

I can update the PR to that scheme if you like?

[guruofquality]

A rule for good cmake options that gets handed around is: do exactly as selected with errors instead of fallbacks.

I.e. -DENABLE_AVAHI=OFF should never include Avahi; -DENABLE_AVAHI=ON must include Avahi or error (not the case currently); and the somewhat accepted middle ground would be a third -DENABLE_AVAHI=AUTO choice.

I can update the PR to that scheme if you like?

Yea I think your right. And I think the change as it is now makes sense to do.

I also like the idea of AUTO/OFF/ON. I wonder if that plays badly with option() and how if() cmake treats variables as booleans. I feel like I rarely see this ON must succeed vs AUTO use if found. Like is there a convention here to follow? I feel like thats not the last time something like this will come up :wink:

[zuckschwerdt]

In my experience it works very well. E.g. (just to illustrate the form)

set(ENABLE_AVAHI AUTO CACHE STRING "Enable Avahi support")
set_property(CACHE ENABLE_AVAHI PROPERTY STRINGS AUTO ON OFF)
if(ENABLE_AVAHI) # AUTO / ON

find_package(Avahi)
if(Avahi_FOUND)
    message(STATUS "Avahi support will be compiled.")
    ADD_DEFINITIONS(-DAVAHI)
elseif(ENABLE_AVAHI STREQUAL "AUTO")
    message(STATUS "Avahi support not found, some features will be disabled.")
else()
    message(FATAL_ERROR "Avahi support not found.")
endif()

else()
    message(STATUS "Avahi support disabled.")
endif()

[zuckschwerdt]

I'd say in general having AUTO (or a similar fallback) is not well liked, esp. as default. It can be a problem when packaging and to reproducibility. There is a good chance one (or e.g. a CI pipeline) won't notice what features are chosen and if some problem has reduced a intended feature set.

OTOH, I quite like the "build whatever I can get here now" option AUTO gives me. But I guess erroring with FOO not found, use -DENABLE_FOO=OFF to disable. and some tries would get me the same result.

[guruofquality]

I see what you mean. Its not great for a package build or CI build to accidentally build features or not build features based on the development environment being different.

My original intention at least was to have the build from source mode just "try its best" to enable features without specifying flags, if CI or build systems need more control, they will just have to code flags into the build scripts. So I think the AUTO scheme you mentioned is pretty good and has a decent balance of tradeoffs with the control when flags are specified.

I'd say go ahead and take over the PR and add that AUTO scheme.

:grin: Over-engineering a PR to disable avahi with a flag :grin:

[zuckschwerdt]

Done. I kept the original commit for reference so this should be Squash merged.

This now also fixes inconsistent case in the cmake module, it honors the case as in FindAvahi.cmake and find_package_handle_standard_args(Avahi, ...) -- i.e. Avahi not AVAHI.

Also I noticed that find_package(SoapySDR "0.8.1" CONFIG) should add REQUIRED so we get a proper fatal error and not something cryptic later on.

(the CI fails are just 404s downloading base files.)

[zuckschwerdt]

@grutz When you have time to test then let us know if this works for your case or is missing something.

[grutz]

Looks great for my case! During build w/o AVAHI libs installed:

...
-- Could NOT find Avahi (missing: AVAHI_LIBRARY-COMMON AVAHI_LIBRARY-CLIENT AVAHI_INCLUDE_DIR)
CMake Warning at common/CMakeLists.txt:68 (message):
  Cannot find Avahi client development files:Avahi is recommended for device
  discovery over mDNS.Please install libavahi-client-dev or equivalent.
...

And on run:

$ server/SoapySDRServer -bind
######################################################
## Soapy Server -- Use any Soapy SDR remotely
######################################################

Server version: 0.6.0-gf375555e
Server UUID: d44ad695-e20d-1765-8567-1efb007f0101
Launching the server... tcp://[::]:55132
Server bound to [::]:55132
Launching discovery server...
Connecting to DNS-SD daemon...
[WARNING] SoapyRemote compiled without DNS-SD support!
Press Ctrl+C to stop the server

[zuckschwerdt]

You may need to reset and pull this branch (or clone again), the message should have been Could NOT find Avahi not Cannot find Avahi… ;)