Subject: [bug#78546] [PATCH 3/3] services: networking: Add iwd-service-type.

From: Sergey Trofimov <[email-masked]>
Date: Fri, 23 May 2025 14:38:27 +0200

Hi Ludovic,

thanks for the review, I've answered a couple points below

Ludovic Courtès <[email-masked]> writes:

> Hi Sergey,
>
> Sergey Trofimov <[email-masked]> writes:
>
>> * gnu/services/networking.scm (iwd-service-type): New service type.
>> (iwd-configuration), (iwd-settings), (iwd-scan-settings),
>> (iwd-general-settings), (iwd-network-settings): New configuration types.
>> * doc/guix.texi (Networking setup): Document it.
>>
>> Change-Id: I852115b9c6768b3ec4eedb34a7f9e66438bd1429
>
> […]
>
>> +@defvar iwd-service-type
>> +This is the service type to run @acronyml{IWD,Internet Wireless Daemon},
>> +a wireless daemon required to authenticate against encrypted WiFi
>> +networks.
>
> Am I right that this conflicts with wpa-supplicant, which is what
> NetworkManager uses?  Should this be mentioned?
>

Sure, I'll also add 'wireless-daemon' to both iwd and wpa-supplicant
shepherd's provisions. This way a conflict in the config would result in
an error message.

>
>> +@deftp {Data Type} iwd-scan-settings
>> +Available @code{iwd-scan-settings} fields are:
>> +
>> +@table @asis
>> +@item @code{disable-periodic-scan} (type: maybe-boolean)
>
> As a rule of thumb, I recommend avoiding negatives for Booleans; also
> add a question mark.
>

These are the names of iwd settings, see man 5 iwd.config. I will add
question marks for booleans, but I'd rather leave the names as they're
defined by iwd authors.

>
> So this would become ‘enable-periodic-scan?’, defaulting to #t.
>
> And it can be ‘boolean’ rather than ‘maybe-boolean’ (the latter would
> default to “unset”, whose meaning isn’t clear here).
>
> Same for ‘disable-roaming-scan’.
>
>> +@item @code{initial-periodic-scan-interval} (type: maybe-number)
>> +The initial periodic scan interval upon disconnect (in seconds).
>> +
>> +@item @code{maximum-periodic-scan-interval} (type: maybe-number)
>> +The maximum periodic scan interval (in seconds).
>
> I would tend to provide a known default value and make it ‘number’
> instead of ‘maybe-number’.
>

I agree, I'll change that.