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

[esben@xxxxxxxxxx]

Kent Gibson <warthog618@xxxxxxxxx> writes:

> 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.

I guess I will have to do that. Although I don't agree that I am out of
scope. I just want to do exactly what you have described is in scope for
gpioset. I just don't want the prompt when not using a tty, and the
reason for the prompt being there is to make the test work, not for a
real-world use-case.  Anyway, I can do my own thing.  No problem.

>> > 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.

Ok.

What I am saying is if you actually do manage to run gpioset with stdin
connected to a fifo, and the fifo not having any writers, you will end
up eating up the cpu in a small busy loop.

Because of the problem you describe, you just haven't gotten to that
point though.

> 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)

I haven't really investigated that.  I just made the process running
gpioset hold a dummy writer open to the fifo.

> 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?

Yes, I could do that.  But having an extra process running, and managing
to keep that alive...  If I need to carry a tiny out-of-tree patch to
avoid that, I will do that.

>> 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.

Ok.

That I guess leaves me with no options than working around gpioset,
using filters and what else is needed to do what I need.
Or out-of-tree patching.

/Esben