Re: [ptxdist] [PATCH] gpsd: fix gpsdctl@.service

[Michael Olbrich <m.olbrich@pengutronix.de>]

On Mon, Aug 03, 2020 at 10:35:52AM +0200, Ladislav Michl wrote:
> On Mon, Aug 03, 2020 at 08:36:06AM +0200, Michael Olbrich wrote:
> > On Sun, Aug 02, 2020 at 07:46:09PM +0200, Ladislav Michl wrote:
> > > gpsdctl@.service is supposed to call gpsdctl which is not installed
> > > at all. Also fix gpsctl vs gpsdctl confusion.
> > > 
> > > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> > > ---
> > >  projectroot/usr/lib/systemd/system/gpsdctl@.service | 4 ++--
> > >  rules/gpsd.in                                       | 9 +++++++++
> > >  rules/gpsd.make                                     | 8 +++++---
> > >  3 files changed, 16 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/projectroot/usr/lib/systemd/system/gpsdctl@.service b/projectroot/usr/lib/systemd/system/gpsdctl@.service
> > > index 24d291408..6266fd8b8 100644
> > > --- a/projectroot/usr/lib/systemd/system/gpsdctl@.service
> > > +++ b/projectroot/usr/lib/systemd/system/gpsdctl@.service
> > > @@ -8,5 +8,5 @@ After=dev-%i.device
> > >  Type=oneshot
> > >  Environment="GPSD_SOCKET=/var/run/gpsd.sock"
> > >  RemainAfterExit=yes
> > > -ExecStart=/usr/bin/gpsdctl add /dev/%I
> > > -ExecStop=/usr/bin/gpsdctl remove /dev/%I
> > > +ExecStart=/usr/sbin/gpsdctl add /dev/%I
> > > +ExecStop=/usr/sbin/gpsdctl remove /dev/%I
> > > diff --git a/rules/gpsd.in b/rules/gpsd.in
> > > index b69570af0..b69accac3 100644
> > > --- a/rules/gpsd.in
> > > +++ b/rules/gpsd.in
> > > @@ -260,10 +260,19 @@ menu "install options"
> > >  		  queries with a format that is substantially easier to
> > >  		  parse than the NMEA 0183 emitted by most GPS receivers.
> > >  
> > > +	config GPSD_GPSDCTL
> > > +		bool
> > > +		prompt "gpdsctl"
> > > +		help
> > > +		  gpdsctl is a tool for sending commands to an instance of
> > > +		  gpsd over its control socket to add or remove devices from
> > > +		  the daemon's device list.
> > > +
> > >  	config GPSD_SYSTEMD_UNIT
> > >  		bool
> > >  		default y
> > >  		depends on INITMETHOD_SYSTEMD && GPSD_GPSD
> > > +		select GPSD_GPSDCTL
> > >  		prompt "systemd unit files for gpsd"
> > >  
> > >  	config GPSD_GPSD_ARGS
> > > diff --git a/rules/gpsd.make b/rules/gpsd.make
> > > index 7f7e369f3..48bb643bc 100644
> > > --- a/rules/gpsd.make
> > > +++ b/rules/gpsd.make
> > > @@ -43,7 +43,8 @@ GPSD_PROGS-$(PTXCONF_GPSD_GPSMON)	+= gpsmon
> > >  GPSD_PROGS-$(PTXCONF_GPSD_NTPSHMMON)	+= ntpshmmon
> > >  GPSD_PROGS-$(PTXCONF_GPSD_PPSCHECK)	+= ppscheck
> > >  
> > > -GPSD_BUILD_CLIENTS := $(if $(strip $(GPSD_PROGS-y)),yes,no)
> > > +GPSD_BUILD_CLIENTS := $(call ptx/ifdef, PTXCONF_GPSD_GPSDCTL, yes, \
> > > +			$(if $(strip $(GPSD_PROGS-y)), yes, no))
> > >  
> > >  # Python programs
> > >  GPSD_PROGS-$(PTXCONF_GPSD_GEGPS)	+= gegps
> > > @@ -160,6 +161,9 @@ $(STATEDIR)/gpsd.targetinstall:
> > >  ifdef PTXCONF_GPSD_GPSD
> > >  	@$(call install_copy, gpsd, 0, 0, 0755, -, /usr/sbin/gpsd)
> > >  endif
> > > +ifdef PTXCONF_GPSD_GPSDCTL
> > > +	@$(call install_copy, gpsd, 0, 0, 0755, -, /usr/sbin/gpsdctl)
> > > +endif
> > >  ifdef PTXCONF_GPSD_SYSTEMD_UNIT
> > >  	@$(call install_alternative, gpsd, 0, 0, 644, \
> > >  		/usr/lib/systemd/system/gpsd.service)
> > > @@ -172,11 +176,9 @@ ifdef PTXCONF_GPSD_SYSTEMD_UNIT
> > >  		/usr/lib/systemd/system/gpsd.socket)
> > >  	@$(call install_link, gpsd, ../gpsd.socket, \
> > >  		/usr/lib/systemd/system/sockets.target.wants/gpsd.socket)
> > > -ifdef PTXCONF_GPSD_GPSCTL
> > 
> > So, I don't know a lot about gpsd. So I need some clarification here.
> > 
> > There is a gpsd.service and the gpsdctl@.service template. What do they do?
> > Does gpsd.service do anything useful without gpsdctl@.service?
> 
> gpsd.service just starts gpsd as a daemon on a device passed as its
> argument (GPSD_GPSD_ARGS). But GPS receiver could be hotplugged as well
> and that's when gpsdctl@.service comes to action.

So with a static setup gpsdctl@.service and gpsdctl are not needed, right?

> > If yes then we should probably install gpsdctl@.service separately and not
> > add the 'select GPSD_GPSDCTL' dependency above.
> 
> As gpsdctl@.service calls gpsdctl, which is installed once GPSD_GPSDCTL
> selected, that dependency is needed.

Or 'ifdef PTXCONF_GPSD_GPSDCTL' to install gpsdctl@.service only if gpsdctl
is actually installed.

This way we can either run a gpsd.service without gpsdctl at all or use the
two together.

Michael

> > If no then I think the GPSD_GPSDCTL option is not needed and it can be
> > gpsdctl can be installed for GPSD_GPSD.
> 
> This option is usefull if developer wishes to talk to running gpsd using
> gpsdctl at his own.

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de