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.

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

Channel label support #1229

Merged
merged 9 commits into from
Jan 27, 2025
1 change: 1 addition & 0 deletions bindings/cpp/iiopp.h
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ class Channel
Device device() const;
cstr id() const {return iio_channel_get_id(p);}
optstr name() const { return impl::opt(iio_channel_get_name(p));}
optstr label() const { return impl::opt(iio_channel_get_label(p));}
bool is_output() const { return iio_channel_is_output(p);}
bool is_scan_element() const { return iio_channel_is_scan_element(p);}
unsigned int attrs_count() const {return iio_channel_get_attrs_count(p);}
Expand Down
9 changes: 9 additions & 0 deletions bindings/csharp/Channel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,9 @@ public struct DataFormat
[DllImport(IioLib.dllname, CallingConvention = CallingConvention.Cdecl)]
private static extern IntPtr iio_channel_get_name(IntPtr chn);

[DllImport(IioLib.dllname, CallingConvention = CallingConvention.Cdecl)]
private static extern IntPtr iio_channel_get_label(IntPtr chn);

[DllImport(IioLib.dllname, CallingConvention = CallingConvention.Cdecl)]
private static extern uint iio_channel_get_attrs_count(IntPtr chn);

Expand Down Expand Up @@ -216,6 +219,9 @@ private static extern uint iio_channel_write(IntPtr chn,
/// <summary>The name of this channel.</summary>
public readonly string name;

/// <summary>The label of this channel.</summary>
public string label { get; private set; }

/// <summary>An identifier of this channel.</summary>
/// <remarks>It is possible that two channels have the same ID,
/// if one is an input channel and the other is an output channel.</remarks>
Expand Down Expand Up @@ -273,6 +279,9 @@ internal Channel(Device dev, IntPtr chn)
name = Marshal.PtrToStringAnsi(name_ptr);
}

IntPtr label_ptr = iio_channel_get_label(this.chn);
label = label_ptr == IntPtr.Zero ? "" : Marshal.PtrToStringAnsi(label_ptr);

id = Marshal.PtrToStringAnsi(iio_channel_get_id(this.chn));
output = iio_channel_is_output(this.chn);
scan_element = iio_channel_is_scan_element(this.chn);
Expand Down
10 changes: 10 additions & 0 deletions bindings/python/iio.py
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,10 @@ class ChannelType(Enum):
_c_get_name.restype = c_char_p
_c_get_name.argtypes = (_ChannelPtr,)

_c_get_label = _lib.iio_channel_get_label
_c_get_label.restype = c_char_p
_c_get_label.argtypes = (_ChannelPtr,)

_c_is_output = _lib.iio_channel_is_output
_c_is_output.restype = c_bool
_c_is_output.argtypes = (_ChannelPtr,)
Expand Down Expand Up @@ -845,6 +849,9 @@ def __init__(self, dev, _channel):
self._output = _c_is_output(self._channel)
self._scan_element = _c_is_scan_element(self._channel)

label_raw = _c_get_label(self._channel)
self._name = label_raw.decode("ascii") if label_raw is not None else None

def read(self, block, raw=False):
"""
Extract the samples corresponding to this channel from the given iio.Block object.
Expand Down Expand Up @@ -892,6 +899,9 @@ def write(self, block, array, raw=False):
name = property(
lambda self: self._name, None, None, "The name of this channel.\n\ttype=str"
)
label = property(
lambda self: self._label, None, None, "The label of this channel.\n\ttype=str"
)
attrs = property(
lambda self: {attr.name: attr for attr in [
Attr(self, _c_get_attr(self._channel, x))
Expand Down
14 changes: 14 additions & 0 deletions channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,14 @@ ssize_t iio_snprintf_channel_xml(char *ptr, ssize_t len,
iio_update_xml_indexes(ret, &ptr, &len, &alen);
}

if (chn->label) {
ret = iio_snprintf(ptr, len, " label=\"%s\"", chn->label);
if (ret < 0)
return ret;

iio_update_xml_indexes(ret, &ptr, &len, &alen);
}

ret = iio_snprintf(ptr, len, " type=\"%s\" >", chn->is_output ? "output" : "input");
if (ret < 0)
return ret;
Expand Down Expand Up @@ -318,6 +326,11 @@ const char * iio_channel_get_name(const struct iio_channel *chn)
return chn->name;
}

const char * iio_channel_get_label(const struct iio_channel *chn)
{
return chn->label;
}

bool iio_channel_is_output(const struct iio_channel *chn)
{
return chn->is_output;
Expand Down Expand Up @@ -420,6 +433,7 @@ void free_channel(struct iio_channel *chn)
{
iio_free_attrs(&chn->attrlist);
free(chn->name);
free(chn->label);
free(chn->id);
free(chn);
}
Expand Down
2 changes: 1 addition & 1 deletion context.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ static const char xml_header[] = "<?xml version=\"1.0\" encoding=\"utf-8\"?>"
"version-minor CDATA #REQUIRED version-git CDATA #REQUIRED description CDATA #IMPLIED>"
"<!ATTLIST context-attribute name CDATA #REQUIRED value CDATA #REQUIRED>"
"<!ATTLIST device id CDATA #REQUIRED name CDATA #IMPLIED label CDATA #IMPLIED>"
"<!ATTLIST channel id CDATA #REQUIRED type (input|output) #REQUIRED name CDATA #IMPLIED>"
"<!ATTLIST channel id CDATA #REQUIRED type (input|output) #REQUIRED name CDATA #IMPLIED label CDATA #IMPLIED>"
"<!ATTLIST scan-element index CDATA #REQUIRED format CDATA #REQUIRED scale CDATA #IMPLIED>"
"<!ATTLIST attribute name CDATA #REQUIRED filename CDATA #IMPLIED>"
"<!ATTLIST debug-attribute name CDATA #REQUIRED>"
Expand Down
16 changes: 13 additions & 3 deletions device.c
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ struct iio_channel * iio_device_find_channel(const struct iio_device *dev,
continue;

if (!strcmp(chn->id, name) ||
(chn->label && !strcmp(chn->label, name)) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

nothing wrong with the patch but commit subject does not match the style :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you mean that there is an extra tab on this line?

Copy link
Contributor

Choose a reason for hiding this comment

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

no.. the commit title itself. nothing wrong with the code. It's a nitpick but consistency across git log is a good thing. Also good for grepping the history

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I got it now. I need to grab another coffee.

(chn->name && !strcmp(chn->name, name)))
return chn;
}
Expand Down Expand Up @@ -354,12 +355,12 @@ void iio_device_set_pdata(struct iio_device *dev, struct iio_device_pdata *d)
}

struct iio_channel * iio_device_add_channel(struct iio_device *dev, long index,
const char *id, const char *name,
const char *id, const char *name, const char *label,
bool output, bool scan_element,
const struct iio_data_format *fmt)
{
struct iio_channel *chn, **chns;
char *new_id, *new_name = NULL;
char *new_id, *new_name = NULL, *new_label = NULL;
unsigned int i;

chn = zalloc(sizeof(*chn));
Expand All @@ -376,8 +377,15 @@ struct iio_channel * iio_device_add_channel(struct iio_device *dev, long index,
goto err_free_id;
}

if (label) {
new_label = iio_strdup(label);
if (!new_label)
goto err_free_name;
}

chn->id = new_id;
chn->name = new_name;
chn->label = new_label;
chn->dev = dev;
chn->is_output = output;
chn->is_scan_element = scan_element;
Expand All @@ -390,7 +398,7 @@ struct iio_channel * iio_device_add_channel(struct iio_device *dev, long index,

chns = realloc(dev->channels, (dev->nb_channels + 1) * sizeof(*chn));
if (!chns)
goto err_free_name;
goto err_free_label;

chns[dev->nb_channels++] = chn;
dev->channels = chns;
Expand All @@ -407,6 +415,8 @@ struct iio_channel * iio_device_add_channel(struct iio_device *dev, long index,

return chn;

err_free_label:
free(new_label);
err_free_name:
free(new_name);
err_free_id:
Expand Down
5 changes: 4 additions & 1 deletion examples/iio-monitor.c
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ static void * read_thd(void *d)

while (!stop) {
struct iio_device *dev;
const char *name;
const char *name, *label;
int row, col, len, align, line = 2;
unsigned int i, nb_channels, nb = 0;
char buf[1024];
Expand Down Expand Up @@ -208,9 +208,12 @@ static void * read_thd(void *d)

nb++;
name = iio_channel_get_name(chn);
label = iio_channel_get_label(chn);
id = iio_channel_get_id(chn);
if (!name)
name = id;
if (label)
name = label;
unit = id_to_unit(id);

iio_snprintf(buf, sizeof(buf), "</%u></B>%s<!B><!%u>",
Expand Down
2 changes: 1 addition & 1 deletion iio-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ struct iio_channel {
bool is_scan_element;
struct iio_data_format format;
char __format_padding[128 - sizeof(struct iio_data_format)];
char *name, *id;
char *name, *id, *label;
long index;
enum iio_modifier modifier;
enum iio_chan_type type;
Expand Down
5 changes: 3 additions & 2 deletions include/iio/iio-backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,9 @@ iio_context_add_device(struct iio_context *ctx,

__api struct iio_channel *
iio_device_add_channel(struct iio_device *dev, long index,
const char *id, const char *name, bool output,
bool scan_element, const struct iio_data_format *fmt);
const char *id, const char *name, const char *label,
bool output, bool scan_element,
const struct iio_data_format *fmt);

__api int
iio_context_add_attr(struct iio_context *ctx,
Expand Down
16 changes: 11 additions & 5 deletions include/iio/iio.h
Original file line number Diff line number Diff line change
Expand Up @@ -782,14 +782,14 @@ __api __check_ret __pure const struct iio_attr *
iio_device_get_attr(const struct iio_device *dev, unsigned int index);


/** @brief Try to find a channel structure by its name of ID
/** @brief Try to find a channel structure by its ID, label or name
* @param dev A pointer to an iio_device structure
* @param name A NULL-terminated string corresponding to the name or the ID of
* the channel to search for
* @param name A NULL-terminated string corresponding to ID, label or name
* of the channel to search for
* @param output True if the searched channel is output, False otherwise
* @return On success, a pointer to an iio_channel structure
* @return If the name or ID does not correspond to any known channel of the
* given device, NULL is returned */
* @return If the ID, label or name does not correspond to any known channel of
* the given device, NULL is returned */
__api __check_ret __pure struct iio_channel * iio_device_find_channel(
const struct iio_device *dev, const char *name, bool output);

Expand Down Expand Up @@ -876,6 +876,12 @@ __api __check_ret __pure const char * iio_channel_get_id(const struct iio_channe
* <b>NOTE:</b> if the channel has no name, NULL is returned. */
__api __check_ret __pure const char * iio_channel_get_name(const struct iio_channel *chn);

/** @brief Retrieve the channel label (e.g. <b><i>anglY</i></b>)
* @param chn A pointer to an iio_channel structure
* @return A pointer to a static NULL-terminated string
*
* <b>NOTE:</b> if the channel has no label, NULL is returned. */
__api __check_ret __pure const char * iio_channel_get_label(const struct iio_channel *chn);

/** @brief Return True if the given channel is an output channel
* @param chn A pointer to an iio_channel structure
Expand Down
16 changes: 16 additions & 0 deletions local.c
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,22 @@ static int add_attr_to_channel(struct iio_channel *chn,
union iio_pointer p = { .chn = chn, };
struct iio_attr_list *attrs;
int ret;
size_t lbl_len = strlen("_label");
char label[512];
const char *dev_id;

if (strlen(name) >= lbl_len &&
!strcmp(name + strlen(name) - lbl_len, "_label")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still not convinced the above is not just doing something get_short_attr_name() is already doing... You did not replied to my last comment but unless I'm missing something, if the channel name is in_voltage0_label, that function should return "label" and you can avoid all that pointer arithmetic. If you look at the implementation the whole point of the function is to give the channel short name, meaning, the name after the last _. Is there any other subtle thing I'm missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I forgot to address your concern.

With these changes:

diff --git a/local.c b/local.c
index 2b28912c..28fe25b7 100644
--- a/local.c
+++ b/local.c
@@ -851,6 +851,8 @@ static int add_attr_to_channel(struct iio_channel *chn,
                } else {
                        chn_perror(chn, ret, "Unable to read channel label");
                }
+               fprintf(stderr, "initial_name: %s\n", name);
+               fprintf(stderr, "name_after_get_short: %s\n", get_short_attr_name(chn, name));
                return 0;
        }

I am getting the following:

analog@analog:~/workspace-dan/libiio/build_v1 $ utils/iio_info
initial_name: out_altvoltage1_TX1_I_F2_label
name_after_get_short: TX1_I_F2_label
initial_name: out_altvoltage0_TX1_I_F1_label
name_after_get_short: TX1_I_F1_label
initial_name: out_altvoltage7_TX2_Q_F2_label
name_after_get_short: TX2_Q_F2_label
initial_name: out_altvoltage6_TX2_Q_F1_label
name_after_get_short: TX2_Q_F1_label
initial_name: out_altvoltage3_TX1_Q_F2_label
name_after_get_short: TX1_Q_F2_label
initial_name: out_altvoltage2_TX1_Q_F1_label
name_after_get_short: TX1_Q_F1_label
initial_name: out_altvoltage5_TX2_I_F2_label
name_after_get_short: TX2_I_F2_label
initial_name: out_altvoltage4_TX2_I_F1_label
name_after_get_short: TX2_I_F1_label
initial_name: out_altvoltage1_TX_LO_label
name_after_get_short: TX_LO_label
initial_name: out_altvoltage0_RX_LO_label
name_after_get_short: RX_LO_label
initial_name: in_voltage7_vrefn_label
name_after_get_short: vrefn_label
initial_name: in_voltage1_vccaux_label
name_after_get_short: vccaux_label
initial_name: in_voltage2_vccbram_label
name_after_get_short: vccbram_label
initial_name: in_voltage3_vccpint_label
name_after_get_short: vccpint_label
initial_name: in_voltage0_vccint_label
name_after_get_short: vccint_label
initial_name: in_voltage6_vrefp_label
name_after_get_short: vrefp_label
initial_name: in_voltage5_vccoddr_label
name_after_get_short: vccoddr_label
initial_name: in_voltage4_vccpaux_label
name_after_get_short: vccpaux_label

Which seems that it strips away the input/output information and the channel id.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I was now just getting more familiar with libiio code for populating channels and attrs. Indeed for legacy devices which make use of extended_names (which are now exposed as labels) the short name function won't work.

out_altvoltage1_TX1_I_F2_label - TX1_I_F2 is the extended name and will be seen as the channel name by libiio. This stuff seems to be addressed later on in set_channel_name() where we set the proper attr name. So, even if we handle things later and not in add_attr_to_channel(), not sure it would be simpler. So yeah, looks good as-is

dev_id = iio_device_get_id(iio_channel_get_device(chn));
ret = local_do_read_dev_attr(dev_id, 0, name,
label, sizeof(label), IIO_ATTR_TYPE_CHANNEL);
if (ret > 0)
chn->label = iio_strdup(label);
else
chn_perror(chn, ret, "Unable to read channel label");

return 0;
}
dNechita marked this conversation as resolved.
Show resolved Hide resolved

name = get_short_attr_name(chn, name);

Expand Down
10 changes: 8 additions & 2 deletions utils/iio_attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ static int dump_channel_attributes(const struct iio_device *dev,
}
if (iio_channel_get_name(ch) && quiet == ATTR_VERBOSE)
printf("id '%s', ", iio_channel_get_name(ch));
if (iio_channel_get_label(ch) && quiet == ATTR_VERBOSE)
printf("label '%s', ", iio_channel_get_label(ch));

if (quiet == ATTR_VERBOSE)
printf("attr '%s', ", iio_attr_get_name(attr));
Expand Down Expand Up @@ -537,7 +539,7 @@ int main(int argc, char **argv)
const char *name = iio_device_get_name(dev);
const char *label_or_name = label ? label : name;
const char *label_or_name_or_id = label_or_name ? label_or_name : dev_id;
const char *ch_name;
const char *ch_name, *ch_label;
struct iio_buffer *buffer;
struct iio_channels_mask *mask;
unsigned int nb_attrs, nb_channels, j;
Expand Down Expand Up @@ -625,10 +627,12 @@ int main(int argc, char **argv)
type_name = "input";

ch_name = iio_channel_get_name(ch);
ch_label = iio_channel_get_label(ch);
if (channel_index &&
!str_match(iio_channel_get_id(ch),
argw[channel_index], ignore_case) &&
(!ch_name || !str_match(ch_name,argw[channel_index], ignore_case)))
(!ch_label || !str_match(ch_label,argw[channel_index], ignore_case) &&
(!ch_name || !str_match(ch_name,argw[channel_index], ignore_case))))
continue;

channel_found = true;
Expand All @@ -642,6 +646,8 @@ int main(int argc, char **argv)

if (ch_name)
printf(", id '%s'", ch_name);
if (ch_label)
printf(", label '%s'", ch_label);

printf(" (%s", type_name);

Expand Down
20 changes: 15 additions & 5 deletions utils/iio_info.c
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ static void print_attr(const struct iio_attr *attr,
static void print_channel(const struct iio_channel *chn)
{
const struct iio_data_format *format;
const char *type_name, *name;
const char *type_name, *name, *label;
char sign, repeat[12];

if (iio_channel_is_output(chn))
Expand All @@ -127,14 +127,24 @@ static void print_channel(const struct iio_channel *chn)
type_name = "input";

name = iio_channel_get_name(chn);
label = iio_channel_get_label(chn);
if (colors) {
print_fmt("\t\t\t" FMT_CHN ": " FMT_CHN " (" FMT_CHN,
print_fmt("\t\t\t" FMT_CHN ": " FMT_CHN,
iio_channel_get_id(chn),
name ? name : "", type_name);
name ? name : "");
} else {
printf("\t\t\t%s: %s (%s",
printf("\t\t\t%s: %s",
iio_channel_get_id(chn),
name ? name : "", type_name);
name ? name : "");
}

if (label)
printf(" (label: %s)", label);

if (colors) {
print_fmt(" (" FMT_CHN, type_name);
} else {
printf(" (%s", type_name);
}

if (iio_channel_get_type(chn) == IIO_CHAN_TYPE_UNKNOWN) {
Expand Down
12 changes: 9 additions & 3 deletions xml.c
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ static int create_channel(struct iio_device *dev, xmlNode *node)
xmlAttr *attr;
struct iio_channel *chn;
int err = -ENOMEM;
char *name_ptr = NULL, *id_ptr = NULL;
char *name_ptr = NULL, *label_ptr = NULL, *id_ptr = NULL;
bool output = false;
bool scan_element = false;
long index = -ENOENT;
Expand All @@ -158,6 +158,10 @@ static int create_channel(struct iio_device *dev, xmlNode *node)
name_ptr = iio_strdup(content);
if (!name_ptr)
goto err_free_name_id;
} else if (!strcmp(name, "label")) {
label_ptr = iio_strdup(content);
if (!label_ptr)
goto err_free_name_id;
} else if (!strcmp(name, "id")) {
id_ptr = iio_strdup(content);
if (!id_ptr)
Expand Down Expand Up @@ -190,14 +194,15 @@ static int create_channel(struct iio_device *dev, xmlNode *node)
}
}

chn = iio_device_add_channel(dev, index, id_ptr, name_ptr, output,
scan_element, &format);
chn = iio_device_add_channel(dev, index, id_ptr, name_ptr, label_ptr,
output, scan_element, &format);
if (!chn) {
err = -ENOMEM;
goto err_free_name_id;
}

free(name_ptr);
free(label_ptr);
free(id_ptr);

for (n = node->children; n; n = n->next) {
Expand All @@ -217,6 +222,7 @@ static int create_channel(struct iio_device *dev, xmlNode *node)

err_free_name_id:
free(name_ptr);
free(label_ptr);
free(id_ptr);
return err;
}
Expand Down
Loading