* [PATCH] tools: iio: iio_generic_buffer: add -A to force-enable all channels
@ 2016-10-14 10:18 Eva Rachel Retuya
2016-10-15 14:27 ` Jonathan Cameron
2016-10-17 21:15 ` Peter Meerwald-Stadler
0 siblings, 2 replies; 5+ messages in thread
From: Eva Rachel Retuya @ 2016-10-14 10:18 UTC (permalink / raw)
To: linux-iio, outreachy-kernel
Cc: jic23, knaack.h, lars, pmeerw, Eva Rachel Retuya
If attribute/s is/are already enabled (by default or via scripts or
manual interaction), issuing -a will fail to enable the channels thereby
one has to manually disable the said attribute/s before proceeding with
auto-enabling.
Add a command-line option -A to force-activate all channels regardless
of their current state.
Suggested-by: Alison Schofield <amsfield22@gmail.com>
Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
---
tools/iio/iio_generic_buffer.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
index f39c0e9..84c9888 100644
--- a/tools/iio/iio_generic_buffer.c
+++ b/tools/iio/iio_generic_buffer.c
@@ -247,6 +247,7 @@ void print_usage(void)
fprintf(stderr, "Usage: generic_buffer [options]...\n"
"Capture, convert and output data from IIO device buffer\n"
" -a Auto-activate all available channels\n"
+ " -A Force-activate ALL channels\n"
" -c <n> Do n conversions\n"
" -e Disable wait for event (new data)\n"
" -g Use trigger-less mode\n"
@@ -347,16 +348,22 @@ int main(int argc, char **argv)
int noevents = 0;
int notrigger = 0;
char *dummy;
+ int force = 0;
struct iio_channel_info *channels = NULL;
register_cleanup();
- while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:T:w:", longopts, NULL)) != -1) {
+ while ((c = getopt_long(argc, argv, "aAc:egl:n:N:t:T:w:", longopts,
+ NULL)) != -1) {
switch (c) {
case 'a':
autochannels = AUTOCHANNELS_ENABLED;
break;
+ case 'A':
+ autochannels = AUTOCHANNELS_ENABLED;
+ force = 1;
+ break;
case 'c':
errno = 0;
num_loops = strtoul(optarg, &dummy, 10);
@@ -519,15 +526,15 @@ int main(int argc, char **argv)
"diag %s\n", dev_dir_name);
goto error;
}
- if (num_channels && autochannels == AUTOCHANNELS_ENABLED) {
+ if ((num_channels && autochannels == AUTOCHANNELS_ENABLED) && !force) {
fprintf(stderr, "Auto-channels selected but some channels "
"are already activated in sysfs\n");
fprintf(stderr, "Proceeding without activating any channels\n");
}
- if (!num_channels && autochannels == AUTOCHANNELS_ENABLED) {
- fprintf(stderr,
- "No channels are enabled, enabling all channels\n");
+ if ((!num_channels && autochannels == AUTOCHANNELS_ENABLED) ||
+ ((autochannels == AUTOCHANNELS_ENABLED) && force)) {
+ fprintf(stderr, "Enabling all channels\n");
ret = enable_disable_all_channels(dev_dir_name, 1);
if (ret) {
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] tools: iio: iio_generic_buffer: add -A to force-enable all channels
2016-10-14 10:18 [PATCH] tools: iio: iio_generic_buffer: add -A to force-enable all channels Eva Rachel Retuya
@ 2016-10-15 14:27 ` Jonathan Cameron
2016-10-16 2:13 ` Eva Rachel Retuya
2016-10-17 21:15 ` Peter Meerwald-Stadler
1 sibling, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2016-10-15 14:27 UTC (permalink / raw)
To: Eva Rachel Retuya, linux-iio, outreachy-kernel; +Cc: knaack.h, lars, pmeerw
On 14/10/16 11:18, Eva Rachel Retuya wrote:
> If attribute/s is/are already enabled (by default or via scripts or
> manual interaction), issuing -a will fail to enable the channels thereby
> one has to manually disable the said attribute/s before proceeding with
> auto-enabling.
>
> Add a command-line option -A to force-activate all channels regardless
> of their current state.
>
> Suggested-by: Alison Schofield <amsfield22@gmail.com>
> Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
Seems like a useful addition with minimal 'cost'.
Applied, though there was some fuzz due to a recent patch making
sure the ? was in the argument list, so please check I didn't mess
it up!
Applied to the togreg branch of iio.git - initially pushed out as
testing for the autobuilders to play with it.
Thanks,
Jonathan
> ---
> tools/iio/iio_generic_buffer.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
> index f39c0e9..84c9888 100644
> --- a/tools/iio/iio_generic_buffer.c
> +++ b/tools/iio/iio_generic_buffer.c
> @@ -247,6 +247,7 @@ void print_usage(void)
> fprintf(stderr, "Usage: generic_buffer [options]...\n"
> "Capture, convert and output data from IIO device buffer\n"
> " -a Auto-activate all available channels\n"
> + " -A Force-activate ALL channels\n"
> " -c <n> Do n conversions\n"
> " -e Disable wait for event (new data)\n"
> " -g Use trigger-less mode\n"
> @@ -347,16 +348,22 @@ int main(int argc, char **argv)
> int noevents = 0;
> int notrigger = 0;
> char *dummy;
> + int force = 0;
>
> struct iio_channel_info *channels = NULL;
>
> register_cleanup();
>
> - while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:T:w:", longopts, NULL)) != -1) {
> + while ((c = getopt_long(argc, argv, "aAc:egl:n:N:t:T:w:", longopts,
> + NULL)) != -1) {
> switch (c) {
> case 'a':
> autochannels = AUTOCHANNELS_ENABLED;
> break;
> + case 'A':
> + autochannels = AUTOCHANNELS_ENABLED;
> + force = 1;
> + break;
> case 'c':
> errno = 0;
> num_loops = strtoul(optarg, &dummy, 10);
> @@ -519,15 +526,15 @@ int main(int argc, char **argv)
> "diag %s\n", dev_dir_name);
> goto error;
> }
> - if (num_channels && autochannels == AUTOCHANNELS_ENABLED) {
> + if ((num_channels && autochannels == AUTOCHANNELS_ENABLED) && !force) {
> fprintf(stderr, "Auto-channels selected but some channels "
> "are already activated in sysfs\n");
> fprintf(stderr, "Proceeding without activating any channels\n");
> }
>
> - if (!num_channels && autochannels == AUTOCHANNELS_ENABLED) {
> - fprintf(stderr,
> - "No channels are enabled, enabling all channels\n");
> + if ((!num_channels && autochannels == AUTOCHANNELS_ENABLED) ||
> + ((autochannels == AUTOCHANNELS_ENABLED) && force)) {
> + fprintf(stderr, "Enabling all channels\n");
>
> ret = enable_disable_all_channels(dev_dir_name, 1);
> if (ret) {
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tools: iio: iio_generic_buffer: add -A to force-enable all channels
2016-10-15 14:27 ` Jonathan Cameron
@ 2016-10-16 2:13 ` Eva Rachel Retuya
0 siblings, 0 replies; 5+ messages in thread
From: Eva Rachel Retuya @ 2016-10-16 2:13 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, outreachy-kernel, knaack.h, lars, pmeerw
On Sat, Oct 15, 2016 at 03:27:22PM +0100, Jonathan Cameron wrote:
> On 14/10/16 11:18, Eva Rachel Retuya wrote:
> > If attribute/s is/are already enabled (by default or via scripts or
> > manual interaction), issuing -a will fail to enable the channels thereby
> > one has to manually disable the said attribute/s before proceeding with
> > auto-enabling.
> >
> > Add a command-line option -A to force-activate all channels regardless
> > of their current state.
> >
> > Suggested-by: Alison Schofield <amsfield22@gmail.com>
> > Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
> Seems like a useful addition with minimal 'cost'.
>
> Applied, though there was some fuzz due to a recent patch making
> sure the ? was in the argument list, so please check I didn't mess
> it up!
It looks fine, thanks!
Eva
>
> Applied to the togreg branch of iio.git - initially pushed out as
> testing for the autobuilders to play with it.
>
> Thanks,
>
> Jonathan
> > ---
> > tools/iio/iio_generic_buffer.c | 17 ++++++++++++-----
> > 1 file changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
> > index f39c0e9..84c9888 100644
> > --- a/tools/iio/iio_generic_buffer.c
> > +++ b/tools/iio/iio_generic_buffer.c
> > @@ -247,6 +247,7 @@ void print_usage(void)
> > fprintf(stderr, "Usage: generic_buffer [options]...\n"
> > "Capture, convert and output data from IIO device buffer\n"
> > " -a Auto-activate all available channels\n"
> > + " -A Force-activate ALL channels\n"
> > " -c <n> Do n conversions\n"
> > " -e Disable wait for event (new data)\n"
> > " -g Use trigger-less mode\n"
> > @@ -347,16 +348,22 @@ int main(int argc, char **argv)
> > int noevents = 0;
> > int notrigger = 0;
> > char *dummy;
> > + int force = 0;
> >
> > struct iio_channel_info *channels = NULL;
> >
> > register_cleanup();
> >
> > - while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:T:w:", longopts, NULL)) != -1) {
> > + while ((c = getopt_long(argc, argv, "aAc:egl:n:N:t:T:w:", longopts,
> > + NULL)) != -1) {
> > switch (c) {
> > case 'a':
> > autochannels = AUTOCHANNELS_ENABLED;
> > break;
> > + case 'A':
> > + autochannels = AUTOCHANNELS_ENABLED;
> > + force = 1;
> > + break;
> > case 'c':
> > errno = 0;
> > num_loops = strtoul(optarg, &dummy, 10);
> > @@ -519,15 +526,15 @@ int main(int argc, char **argv)
> > "diag %s\n", dev_dir_name);
> > goto error;
> > }
> > - if (num_channels && autochannels == AUTOCHANNELS_ENABLED) {
> > + if ((num_channels && autochannels == AUTOCHANNELS_ENABLED) && !force) {
> > fprintf(stderr, "Auto-channels selected but some channels "
> > "are already activated in sysfs\n");
> > fprintf(stderr, "Proceeding without activating any channels\n");
> > }
> >
> > - if (!num_channels && autochannels == AUTOCHANNELS_ENABLED) {
> > - fprintf(stderr,
> > - "No channels are enabled, enabling all channels\n");
> > + if ((!num_channels && autochannels == AUTOCHANNELS_ENABLED) ||
> > + ((autochannels == AUTOCHANNELS_ENABLED) && force)) {
> > + fprintf(stderr, "Enabling all channels\n");
> >
> > ret = enable_disable_all_channels(dev_dir_name, 1);
> > if (ret) {
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tools: iio: iio_generic_buffer: add -A to force-enable all channels
2016-10-14 10:18 [PATCH] tools: iio: iio_generic_buffer: add -A to force-enable all channels Eva Rachel Retuya
2016-10-15 14:27 ` Jonathan Cameron
@ 2016-10-17 21:15 ` Peter Meerwald-Stadler
2016-10-19 17:09 ` Eva Rachel Retuya
1 sibling, 1 reply; 5+ messages in thread
From: Peter Meerwald-Stadler @ 2016-10-17 21:15 UTC (permalink / raw)
To: Eva Rachel Retuya; +Cc: linux-iio, outreachy-kernel, jic23, knaack.h, lars
> If attribute/s is/are already enabled (by default or via scripts or
> manual interaction), issuing -a will fail to enable the channels thereby
> one has to manually disable the said attribute/s before proceeding with
> auto-enabling.
>
> Add a command-line option -A to force-activate all channels regardless
> of their current state.
comments below
> Suggested-by: Alison Schofield <amsfield22@gmail.com>
> Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
> ---
> tools/iio/iio_generic_buffer.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
> index f39c0e9..84c9888 100644
> --- a/tools/iio/iio_generic_buffer.c
> +++ b/tools/iio/iio_generic_buffer.c
> @@ -247,6 +247,7 @@ void print_usage(void)
> fprintf(stderr, "Usage: generic_buffer [options]...\n"
> "Capture, convert and output data from IIO device buffer\n"
> " -a Auto-activate all available channels\n"
> + " -A Force-activate ALL channels\n"
> " -c <n> Do n conversions\n"
> " -e Disable wait for event (new data)\n"
> " -g Use trigger-less mode\n"
> @@ -347,16 +348,22 @@ int main(int argc, char **argv)
> int noevents = 0;
> int notrigger = 0;
> char *dummy;
> + int force = 0;
bool instead of int
maybe force_autochannels
>
> struct iio_channel_info *channels = NULL;
>
> register_cleanup();
>
> - while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:T:w:", longopts, NULL)) != -1) {
> + while ((c = getopt_long(argc, argv, "aAc:egl:n:N:t:T:w:", longopts,
> + NULL)) != -1) {
> switch (c) {
> case 'a':
> autochannels = AUTOCHANNELS_ENABLED;
> break;
> + case 'A':
> + autochannels = AUTOCHANNELS_ENABLED;
> + force = 1;
> + break;
> case 'c':
> errno = 0;
> num_loops = strtoul(optarg, &dummy, 10);
> @@ -519,15 +526,15 @@ int main(int argc, char **argv)
> "diag %s\n", dev_dir_name);
> goto error;
> }
> - if (num_channels && autochannels == AUTOCHANNELS_ENABLED) {
> + if ((num_channels && autochannels == AUTOCHANNELS_ENABLED) && !force) {
parenthesis not strictly required/needed
the bracketing is inconsistent, when do you use parenthesis with && and
when not? I suggest to drop parenthesis
> fprintf(stderr, "Auto-channels selected but some channels "
> "are already activated in sysfs\n");
> fprintf(stderr, "Proceeding without activating any channels\n");
> }
>
> - if (!num_channels && autochannels == AUTOCHANNELS_ENABLED) {
> - fprintf(stderr,
> - "No channels are enabled, enabling all channels\n");
> + if ((!num_channels && autochannels == AUTOCHANNELS_ENABLED) ||
> + ((autochannels == AUTOCHANNELS_ENABLED) && force)) {
> + fprintf(stderr, "Enabling all channels\n");
as above
>
> ret = enable_disable_all_channels(dev_dir_name, 1);
> if (ret) {
>
--
Peter Meerwald-Stadler
+43-664-2444418 (mobile)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tools: iio: iio_generic_buffer: add -A to force-enable all channels
2016-10-17 21:15 ` Peter Meerwald-Stadler
@ 2016-10-19 17:09 ` Eva Rachel Retuya
0 siblings, 0 replies; 5+ messages in thread
From: Eva Rachel Retuya @ 2016-10-19 17:09 UTC (permalink / raw)
To: Peter Meerwald-Stadler; +Cc: linux-iio, outreachy-kernel, jic23, knaack.h, lars
On Mon, Oct 17, 2016 at 11:15:53PM +0200, Peter Meerwald-Stadler wrote:
>
> > If attribute/s is/are already enabled (by default or via scripts or
> > manual interaction), issuing -a will fail to enable the channels thereby
> > one has to manually disable the said attribute/s before proceeding with
> > auto-enabling.
> >
> > Add a command-line option -A to force-activate all channels regardless
> > of their current state.
>
> comments below
>
> > Suggested-by: Alison Schofield <amsfield22@gmail.com>
> > Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
> > ---
> > tools/iio/iio_generic_buffer.c | 17 ++++++++++++-----
> > 1 file changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
> > index f39c0e9..84c9888 100644
> > --- a/tools/iio/iio_generic_buffer.c
> > +++ b/tools/iio/iio_generic_buffer.c
> > @@ -247,6 +247,7 @@ void print_usage(void)
> > fprintf(stderr, "Usage: generic_buffer [options]...\n"
> > "Capture, convert and output data from IIO device buffer\n"
> > " -a Auto-activate all available channels\n"
> > + " -A Force-activate ALL channels\n"
> > " -c <n> Do n conversions\n"
> > " -e Disable wait for event (new data)\n"
> > " -g Use trigger-less mode\n"
> > @@ -347,16 +348,22 @@ int main(int argc, char **argv)
> > int noevents = 0;
> > int notrigger = 0;
> > char *dummy;
> > + int force = 0;
>
> bool instead of int
> maybe force_autochannels
>
> >
> > struct iio_channel_info *channels = NULL;
> >
> > register_cleanup();
> >
> > - while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:T:w:", longopts, NULL)) != -1) {
> > + while ((c = getopt_long(argc, argv, "aAc:egl:n:N:t:T:w:", longopts,
> > + NULL)) != -1) {
> > switch (c) {
> > case 'a':
> > autochannels = AUTOCHANNELS_ENABLED;
> > break;
> > + case 'A':
> > + autochannels = AUTOCHANNELS_ENABLED;
> > + force = 1;
> > + break;
> > case 'c':
> > errno = 0;
> > num_loops = strtoul(optarg, &dummy, 10);
> > @@ -519,15 +526,15 @@ int main(int argc, char **argv)
> > "diag %s\n", dev_dir_name);
> > goto error;
> > }
> > - if (num_channels && autochannels == AUTOCHANNELS_ENABLED) {
> > + if ((num_channels && autochannels == AUTOCHANNELS_ENABLED) && !force) {
>
> parenthesis not strictly required/needed
> the bracketing is inconsistent, when do you use parenthesis with && and
> when not? I suggest to drop parenthesis
>
> > fprintf(stderr, "Auto-channels selected but some channels "
> > "are already activated in sysfs\n");
> > fprintf(stderr, "Proceeding without activating any channels\n");
> > }
> >
> > - if (!num_channels && autochannels == AUTOCHANNELS_ENABLED) {
> > - fprintf(stderr,
> > - "No channels are enabled, enabling all channels\n");
> > + if ((!num_channels && autochannels == AUTOCHANNELS_ENABLED) ||
> > + ((autochannels == AUTOCHANNELS_ENABLED) && force)) {
> > + fprintf(stderr, "Enabling all channels\n");
>
> as above
Thanks for the feedback. I submitted a patchset about these suggestions.
However, I cannot fully omit all extra parentheses on that part
otherwise the tool will not compile due to requiring parentheses around
'||'.
Eva
> >
> > ret = enable_disable_all_channels(dev_dir_name, 1);
> > if (ret) {
> >
>
> --
>
> Peter Meerwald-Stadler
> +43-664-2444418 (mobile)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-10-19 17:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-14 10:18 [PATCH] tools: iio: iio_generic_buffer: add -A to force-enable all channels Eva Rachel Retuya
2016-10-15 14:27 ` Jonathan Cameron
2016-10-16 2:13 ` Eva Rachel Retuya
2016-10-17 21:15 ` Peter Meerwald-Stadler
2016-10-19 17:09 ` Eva Rachel Retuya
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.