[Buildroot] [PATCH v4 3/5] package/wpa_supplicant: configure wifi on systemd when enabled

[Arnout Vandecappelle]

On 04/10/2022 13:04, Angelo Compagnucci wrote:
> Configure a default basic wifi setup able to automatically connect
> to the selected access point.

  I think this feature is indeed useful. IIUC, currently wpa_supplicant actually 
isn't started, even though we do install a service file.

> 
> Signed-off-by: Angelo Compagnucci <angelo at amarulasolutions.com>
> ---
>   package/wpa_supplicant/wpa_supplicant.mk | 10 ++++++++++
>   system/Config.in                         |  1 +
>   2 files changed, 11 insertions(+)
> 
> diff --git a/package/wpa_supplicant/wpa_supplicant.mk b/package/wpa_supplicant/wpa_supplicant.mk
> index 60ae290e26..8611fc6577 100644
> --- a/package/wpa_supplicant/wpa_supplicant.mk
> +++ b/package/wpa_supplicant/wpa_supplicant.mk
> @@ -277,6 +277,15 @@ define WPA_SUPPLICANT_ENABLE_WIFI
>   		echo "}"; \
>   	) >> $(TARGET_DIR)/etc/wpa_supplicant.conf
>   endef
> +define WPA_SUPPLICANT_WIFI_INSTALL_INIT_SYSTEMD
> +	$(INSTALL) -m 0755 -d $(TARGET_DIR)/etc/wpa_supplicant/
> +	ln -sf ../wpa_supplicant.conf \
> +		$(TARGET_DIR)/etc/wpa_supplicant/wpa_supplicant-$(BR2_SYSTEM_DHCP).conf

  If the interface-specific service is used rather than the global one, I think 
the idea is to be able to use separate conf files for them. So I think it makes 
more sense to copy than symlink.

  That said, I'm not sure if it's really useful to use the per-interface 
service. The global one should work as well, no? I think the only thing we're 
missing is to enable it for the wifi interface.

  Also, I don't think it's a good idea to reuse BR2_SYSTEM_DHCP for this. There 
are quite a few boards that have both ethernet and wifi and ideally we want both 
to come up. So I think it's worth adding a BR2_SYSTEM_WLAN to specify the wifi 
interface - and perhaps automatically select wpa_supplicant if that option is 
set (although we could just as well use iwd instead of supplicant, of course).

> +endef
> +define WPA_SUPPLICANT_WIFI_INSTALL_INIT_SYSTEMD_PRESET
> +	$(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset wpa_supplicant\@$(BR2_SYSTEM_DHCP).service

  I'm not sure what this is supposed to do. Doesn't it just undo the disable we 
do in 50-wpa_supplicant.preset? Wouldn't it be better to just remove that preset 
somehow?

> +endef
> +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += WPA_SUPPLICANT_WIFI_INSTALL_INIT_SYSTEMD_PRESET
>   endif
>   
>   define WPA_SUPPLICANT_INSTALL_TARGET_CMDS
> @@ -304,6 +313,7 @@ define WPA_SUPPLICANT_INSTALL_INIT_SYSTEMD
>   		$(TARGET_DIR)/usr/lib/systemd/system/wpa_supplicant-wired at .service
>   	$(INSTALL) -D -m 644 $(WPA_SUPPLICANT_PKGDIR)/50-wpa_supplicant.preset \
>   		$(TARGET_DIR)/usr/lib/systemd/system-preset/50-wpa_supplicant.preset
> +	$(WPA_SUPPLICANT_WIFI_INSTALL_INIT_SYSTEMD)
>   endef
>   
>   $(eval $(generic-package))
> diff --git a/system/Config.in b/system/Config.in
> index 647072b965..761a5a95c2 100644
> --- a/system/Config.in
> +++ b/system/Config.in
> @@ -421,6 +421,7 @@ comment "automatic network configuration via DHCP needs ifupdown or busybox or n
>   config BR2_SYSTEM_CONNECT_WIFI
>   	bool "Connect to a default wifi access point"
>   	depends on BR2_SYSTEM_DHCP != "" && BR2_PACKAGE_WPA_SUPPLICANT
> +	select BR2_PACKAGE_WPA_SUPPLICANT_CTRL_IFACE if BR2_PACKAGE_SYSTEMD

  This needs some explanation. I don't see the service files requiring this 
anywhere.

  On the other hand, the global service file does start wpa_supplicant with the 
-u option so it would need dbus.


  Regards,
  Arnout

>   
>   config BR2_SYSTEM_CONNECT_WIFI_SSID
>   	string "Access point SSID"