Re: [RFC PATCH] gpioset: only print prompt when stdout is tty

[Kent Gibson <warthog618@xxxxxxxxx>]

On Wed, May 24, 2023 at 08:30:33AM +0200, esben@xxxxxxxxxx wrote:
> Kent Gibson <warthog618@xxxxxxxxx> writes:
> 
> > On Tue, May 23, 2023 at 03:54:41PM +0200, Esben Haabendal wrote:
> >> When gpioset interactive mode is used as intended, as a human controlled
> >> interface, stdout should be a tty.
> >> 
> >
> > Yeah, no, the interactive mode is also intended to be script driven -
> > checkout the test suite, gpio-tools-tests.bat, as an example of it being
> > driven using a coproc from bash.
> > 
> > Removing the prompt would break the handshaking with the controlling
> > script - that is how it determines the slave process is up.
> 
> I see.  I use a process supervisor, which should ensure that the gpioset
> process stays alive.  And if a client writes to the fifo while the
> process is shortly down, it will pick up the request when it starts up.
> 
> A proper gpio daemon would of-course need a request-reply mechanism, so
> the requester can know if the request succeeded.  But that obviously is
> something slightly more involved than removing a single printf() call.
> 

It isn't intended to be a "proper daemon".  It is a cheap and cheerful
option to give something close to the sysfs "echo 1 > /some/sysfs/line",
which doesn't give feedback either.

As you said in your patch:
"a really simple deamon for controlling GPIOs by connecting it to a FIFO"

> > I'll try running your patch through the test suite tommorrow, but I'm
> > pretty sure it will break it - IIRC the code you removed was put there
> > precisely to get the test suite to run.
> >
> > Have you tried running the test suite?
> 
> Yes, I have now.  And I see that they fail with my RFC PATCH.  The use
> of coproc is obviously not compatible with it.
> 
> But I cannot help feeling that the use of coproc to drive a
> command-prompt interface, while well suited for writing a test for such
> an prompt based interactive interface, it is not how you would want to
> talk with a daemon.
> 

Yeah, it isn't a whole load of fun, but it isn't intended as a full on
daemon.  It is an option that was added in v2 so you CAN now write a
shell script that can request lines and change them as necessary - without
releasing them.  It might not be pleasant but now it is possible.

If that doesn't suit you then look for another solution as you are now
beyond the scope that gpioset was intended for.

> > This works for me as a simple daemon script:
> >
> > #!/bin/bash
> >
> > pipe=/tmp/gpiosetd
> >
> > mkfifo $pipe
> >
> > trap "rm -f $pipe" EXIT
> >
> > # as bash will block until something is written to the pipe...
> > echo "" > $pipe &
> 
> I believe this is not just needed because of bash.  If you don't have a
> writer on the fifo, the gpioset will end up in a busy loop in readline
> until a writer appear, spamming a prompt out on output while eating up
> 100% cpu.
> 

I don't see that.

What I see is that bash blocks until something writes to the fifo - not
even launching gpioset until that happens.
That is typically not what you want - you want the line requested and
set NOW, and you can update it later through the fifo.
The echo is just there to get bash over the hump.
(btw, if there is a better way I would love to know it)

With the named fifo, as used here, gpioset will start, request and set
the line, and then will block until something writes to the fifo.

> > gpioset -i GPIO23=0 < $pipe > /dev/null
> >
> > Does that not work for you?
> 
> That is basically what I do.  Just output directed to a log file
> (actually, a pipe to a process writing to rotated log files) instead of
> /dev/null, and then no prompt noise in the log files.
> 

So redirect stdout through a filter to remove the prompt?

> Anyway, what about adding a new CLI option. Either something like '-I'
> for no-prompt interactive mode, or '-n' to be used with '-i' for the
> same?
> 

I'm not keen on adding options to gpioset to massage the output for
different use cases - there are already better tools for that.

Cheers,
Kent.