Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Better debug macros and iio_info output #1086

Merged
merged 14 commits into from
Dec 4, 2023
Merged

Better debug macros and iio_info output #1086

merged 14 commits into from
Dec 4, 2023

Conversation

pcercuei
Copy link
Contributor

  • Update the debug macros to contain the device/channel IDs in the format;
  • Accept negative values in iio_strerror, to simplify its usage and prevent the very common mistake of passing a negative value;
  • Print errors in-line in iio_info, so that the output looks much better and less "hashed";
  • Colorize the iio_info output, so that it's much easier to visually extract information.

@pcercuei
Copy link
Contributor Author

Obligatory screenshots:
Capture d’écran du 2023-11-16 19-20-46
Capture d’écran du 2023-11-16 19-21-18

@pcercuei pcercuei force-pushed the pcercuei/better-debug branch from 63cfbaf to 4f0e335 Compare November 16, 2023 18:24
@rgetz
Copy link
Contributor

rgetz commented Nov 20, 2023

did you want to merge in something similar to #994 at the same time?

@pcercuei
Copy link
Contributor Author

@rgetz done.

@pcercuei pcercuei force-pushed the pcercuei/better-debug branch from dc76fb3 to 5b7d42a Compare November 21, 2023 13:13
@rgetz
Copy link
Contributor

rgetz commented Nov 21, 2023

Looks good to me - thanks

@rgetz rgetz mentioned this pull request Nov 21, 2023
Closed
@pcercuei pcercuei force-pushed the pcercuei/better-debug branch from 5b7d42a to 12b5134 Compare November 21, 2023 15:17
@pcercuei
iio-debug.h: Print device/channel IDs in their print macros
4d861f0 
When using the device and channel-specific debug macros, prefix the
messages with their respective IDs automatically.

For instance, with a device named "iio-dev" and a channel "voltage0",
the following two calls:

  dev_warn(dev, "invalid value\n");
  chn_info(chn, "ready\n");

will result in the following being printed:

  WARNING: iio-dev: invalid value
  iio-dev: voltage0: ready

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei
utilities: Accept negative values in iio_strerror
49f199f 
strerror() is annoying, as it takes positive error codes while those are
traditionally negative on most programs.

Work around this issue in our iio_strerror() function by accepting both
positive and negative error codes.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei pcercuei force-pushed the pcercuei/better-debug branch from 12b5134 to 3197bcb Compare November 22, 2023 12:45
@rgetz
Copy link
Contributor

rgetz commented Nov 27, 2023

did I understand from the comment you made in #1081, that you wanted to add support for events in iio_info in this PR or a separate one?

@pcercuei
iio_info: Factorize code
c1033ee 
Factorize the code that prints attributes into its own function.
Also put the code that prints channels info into its own function.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei
iio_info: Print errors in-line
f0813bd 
Instead of printing errors to the error output, print them in the
iio_info output. Otherwise the output feels disrupted, and it is harder
to see where the errors originally appeared.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei pcercuei force-pushed the pcercuei/better-debug branch 2 times, most recently from 2edcc92 to 3d753e3 Compare November 27, 2023 17:19
@pcercuei
iio_info: Color output
11c0b0f 
When running in a terminal, use a colored output.
The errors, device names, channel names, and attributes are colored in
red, green, yellow and blue respectively.

This makes it much faster to visually extract information from the
iio_info output.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei
iio_info: Advertise support for IIO events
b930fe7 
When IIO events are available for a given IIO device, add a "events
supported" flag.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei
Move library init/exit to its own library.c file
d88344a 
This code was previously inside xml.c. However, we do want to
initialize/deinitialize other things, so it makes more sense to move it
to its own library.c file.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei
iio.h: Add timestamp_level field to iio_context_params
de65137 
Under this log level (excluded), messages are sent without timestamp;
above this log level (included), messages are sent with a timestamp.
If set to LEVEL_NOLOG, messages at all log levels are sent without a
timestamp. If zero, defaults to LEVEL_DEBUG.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei
Add support for timestamps in messages
e044291 
Add support for printing a timestamp before each message. This can be
configured using the 'timestamp_level' field of the iio_context_params
structure. By default, only messages of the LEVEL_DEBUG log level have
timestamps.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei
iio.h: Add padding at the end of iio_context_params
7aa5fd2 
This will enable us to add more fields after Libiio v1.0 without
breaking backwards and forward compatibility.

The 'timeout_ms' field is also moved further down so that it packs
better into the struct - making it a bit smaller on 64-bit systems.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei
attr: Simplify debug messages
5b7ab17 
The fact that we're using dev_dbg() and chn_dbg() means that we are
already printing the device and channel names; so we don't need to print
them in the message itself.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei
iiod: Fix handling of error code for init_usb_daemon()
e74b766 
The init_usb_daemon() returns a valid error code directly, which does
not have to be retrieved from the errno variable.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei
iiod: Switch to use the new Libiio debug macros
64a4a93 
Some error messages were converted to use IIO_PERROR() so that they also
print the error message and code.

This opens the door to have --quiet and --verbose (or --debug) options
to IIOD, since Libiio allows to control the verbosity at runtime.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei
CMake: Add option to specify the maximum log level
b23658d 
This makes sense for embedded, as compiling with a lower maximum log
level means that all strings that correspond to the higher log levels
will be automatically discarded by the compiler, resulting in a smaller
binary.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei pcercuei force-pushed the pcercuei/better-debug branch from 3d753e3 to b23658d Compare November 27, 2023 17:31
@pcercuei
Copy link
Contributor Author

did I understand from the comment you made in #1081, that you wanted to add support for events in iio_info in this PR or a separate one?

Done.

rgetz
rgetz approved these changes Nov 27, 2023
View reviewed changes
Copy link
Contributor

@rgetz rgetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks - looks good to me.

library.c
*
* Copyright (C) 2023 Analog Devices, Inc.
* Author: Paul Cercueil <paul.cercueil@analog.com>
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that this is in a separate file, I don't know if you want to add machinery to print out a banner, like all good LGPL things are supposed to?

c) If the modified program normally reads commands interactively when run, you must cause it, when started running for such interactive use in the most ordinary way, to print or display an announcement including an appropriate copyright notice and a notice that there is no warranty (or else, saying that you provide a warranty) and that users may redistribute the program under these conditions, and telling the user how to view a copy of this License.

I think this is why this exists:
https://github.com/bminor/glibc/blob/master/csu/version.c

and then is connected at :
https://github.com/bminor/glibc/blob/master/Makerules#L597-L598

@rgetz
Copy link
Contributor

rgetz commented Nov 29, 2023

I guess this is just waiting for @mhennerich or @tfcollins to review?

@pcercuei
Copy link
Contributor Author

Yes...

mhennerich
mhennerich approved these changes Dec 4, 2023
View reviewed changes
Copy link
Contributor

@mhennerich mhennerich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pcercuei pcercuei merged commit cc65317 into main Dec 4, 2023
24 checks passed
@pcercuei pcercuei deleted the pcercuei/better-debug branch December 4, 2023 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhennerich mhennerich mhennerich approved these changes

@rgetz rgetz rgetz approved these changes

@tfcollins tfcollins Awaiting requested review from tfcollins

@nunojsa nunojsa Awaiting requested review from nunojsa

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pcercuei @rgetz @mhennerich