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

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

[Esben Haabendal]

When gpioset interactive mode is used as intended, as a human controlled
interface, stdout should be a tty.

By leaving out the prompt when stdout is not a tty, gpioset interactive mode can
be used as a really simple deamon for controlling GPIOs by connecting it to a
FIFO.

Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 tools/gpioset.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/tools/gpioset.c b/tools/gpioset.c
index 9dc5aeb8b286..a1ca211febd7 100644
--- a/tools/gpioset.c
+++ b/tools/gpioset.c
@@ -742,13 +742,12 @@ static void interact(struct gpiod_line_request **requests,
 {
 	int num_words, num_lines, max_words, period_us, i;
 	char *line, **words, *line_buf;
-	bool done, stdout_is_tty;
+	bool done;
 
 	stifle_history(20);
 	rl_attempted_completion_function = tab_completion;
 	rl_basic_word_break_characters = " =\"";
 	completion_context = resolver;
-	stdout_is_tty = isatty(1);
 
 	max_words = resolver->num_lines + 1;
 	words = calloc(max_words, sizeof(*words));
@@ -757,12 +756,9 @@ static void interact(struct gpiod_line_request **requests,
 
 	for (done = false; !done;) {
 		/*
-		 * manually print the prompt, as libedit doesn't if stdout
-		 * is not a tty.  And fflush to ensure the prompt and any
-		 * output buffered from the previous command is sent.
+		 * fflush to ensure the prompt and any output buffered from the
+		 * previous command is sent.
 		 */
-		if (!stdout_is_tty)
-			printf(PROMPT);
 		fflush(stdout);
 
 		line = readline(PROMPT);
-- 
2.40.1

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

[Kent Gibson]

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

> By leaving out the prompt when stdout is not a tty, gpioset interactive mode can
> be used as a really simple deamon for controlling GPIOs by connecting it to a
> FIFO.
> 

It can do that already - just direct the output to /dev/null.
Which you would need in your case anyway - the prompt isn't the only
output - try piping a get command to your daemon and see what happens.

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 &
gpioset -i GPIO23=0 < $pipe > /dev/null

Does that not work for you?

Cheers,
Kent.

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

[esben]

Kent Gibson <warthog618@gmail.com> 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.

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

>> By leaving out the prompt when stdout is not a tty, gpioset interactive mode can
>> be used as a really simple deamon for controlling GPIOs by connecting it to a
>> FIFO.
>
> It can do that already - just direct the output to /dev/null.

If I direct the output to /dev/null, I cannot see what good the prompt
does.  I am directing the output to a log file, and by sending "get"
commands to gpioset, I end up with a log of the gpio states.

> Which you would need in your case anyway - the prompt isn't the only
> output - try piping a get command to your daemon and see what happens.

I know.

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

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

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?

This would make all existing usage of gpioset work just as it is now,
but allowing use without a prompt when that is desired?

/Esben

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

[Kent Gibson]

On Wed, May 24, 2023 at 08:30:33AM +0200, esben@geanix.com wrote:
> Kent Gibson <warthog618@gmail.com> 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.

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

[esben]

Kent Gibson <warthog618@gmail.com> writes:

> On Wed, May 24, 2023 at 08:30:33AM +0200, esben@geanix.com wrote:
>> Kent Gibson <warthog618@gmail.com> 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

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

[Kent Gibson]

On Wed, May 24, 2023 at 09:53:39AM +0200, esben@geanix.com wrote:
> Kent Gibson <warthog618@gmail.com> writes:
> 
> > On Wed, May 24, 2023 at 08:30:33AM +0200, esben@geanix.com wrote:
> >> Kent Gibson <warthog618@gmail.com> writes:
> >> 
> >
> > 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.
> 

Not just for testing.

In the real world the prompt is there so the controlling script can tell
a command is completed - same as a human would.

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

No, that is wrong. I mean I've tested it. Just now. Again. To be sure.

The only reason it would spin is if you connect it to a pipe that always
indicates it is ready to read.  And the named fifo doesn't.

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

Yeah, I don't know what you are doing, as you haven't shared details, but
if it beahves as you say then it obviously broken.
But that is not due to gpioset.

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

If that is easier for you.

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

You missed writing your own daemon. Or asking ChatGPT.
But you do have options.

Have a good one.

Cheers,
Kent.

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

[esben]

Kent Gibson <warthog618@gmail.com> writes:

> On Wed, May 24, 2023 at 09:53:39AM +0200, esben@geanix.com wrote:
>> Kent Gibson <warthog618@gmail.com> writes:
>> 
>> > On Wed, May 24, 2023 at 08:30:33AM +0200, esben@geanix.com wrote:
>> >> Kent Gibson <warthog618@gmail.com> writes:
>> >> 
>> >
>> > 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.
>> 
>
> Not just for testing.
>
> In the real world the prompt is there so the controlling script can tell
> a command is completed - same as a human would.

And still you say that I should probably redirect it to /dev/null ...

>> >> > 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.
>
> No, that is wrong. I mean I've tested it. Just now. Again. To be sure.
>
> The only reason it would spin is if you connect it to a pipe that always
> indicates it is ready to read.  And the named fifo doesn't.

So when you call read(2) on a named pipe opened in blocking mode, but
without any writers, you don't simply get an immediate return with 0,
indicating EOF?  That is what I saw when I tested it previously.
If not, I probably just messed up back then.  Sorry about that.

>> > 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.
>
> Yeah, I don't know what you are doing, as you haven't shared details, but
> if it beahves as you say then it obviously broken.
> But that is not due to gpioset.
>
>> > 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.
>
> If that is easier for you.
>
>> >> 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.
>
> You missed writing your own daemon. Or asking ChatGPT.
> But you do have options.

Yes, you always have options, and I will go with one of them. I will
probably not go with ChatGPT though ;)

/Esben

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

[Kent Gibson]

On Wed, May 24, 2023 at 10:35:53AM +0200, esben@geanix.com wrote:
> Kent Gibson <warthog618@gmail.com> writes:
> 
> > On Wed, May 24, 2023 at 09:53:39AM +0200, esben@geanix.com wrote:
> >> Kent Gibson <warthog618@gmail.com> writes:
> >> 
> >> 
> >
> > Not just for testing.
> >
> > In the real world the prompt is there so the controlling script can tell
> > a command is completed - same as a human would.
> 
> And still you say that I should probably redirect it to /dev/null ...
> 

Those are two different use cases.
Redirect for the simple daemon/fifo, using the prompt with the coproc.

> 
> So when you call read(2) on a named pipe opened in blocking mode, but
> without any writers, you don't simply get an immediate return with 0,
> indicating EOF?  That is what I saw when I tested it previously.
> If not, I probably just messed up back then.  Sorry about that.
> 

gpioset doesn't do the open or read on the fifo itself, bash does.
gpioset just reads from its stdin, and that blocks.

Have you actually tried my script?

> > You missed writing your own daemon. Or asking ChatGPT.
> > But you do have options.
> 
> Yes, you always have options, and I will go with one of them. I will
> probably not go with ChatGPT though ;)
> 

Where do you think I get my answers ;-)?

Cheers,
Kent.