On Mon, 2 Oct 2023 11:58:17 -0500 David Lechner <dlechner@xxxxxxxxxxxx> wrote: > On Sat, Sep 30, 2023 at 11:00 AM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > > > On Fri, 29 Sep 2023 12:23:31 -0500 > > David Lechner <dlechner@xxxxxxxxxxxx> wrote: > > > > > From: David Lechner <david@xxxxxxxxxxxxxx> > > > > > > From: David Lechner <dlechner@xxxxxxxxxxxx> > > > > > > When reading the position and velocity on the AD2S1210, there is also a > > > 3rd byte following the two data bytes that contains the fault flag bits. > > > This patch adds support for reading this byte and generating events when > > > faults occur. > > > > > > The faults are mapped to various channels and event types in order to > > > have a unique event for each fault. > > > > > > Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx> > > > > Use of x and y modifiers is a little odd. What was your reasoning? > > Was it just that there was a X_OR_Y modifier? If so, don't use that! > > It seemed like a good idea at the time, but it's not nice to deal with > > and requires a channel with that modifier to hang the controls off > > + make sure userspace expects that event code. > > > Regarding the point about "requires a channel with that modifier to > hang the controls off...". Although that comment was about modifiers, > does it also apply in general. > > There are several fault events that don't have any configurable > parameters, namely _sine/cosine inputs clipping_ and _velocity exceeds > max tracking rate_. So there won't be any attributes that contain the > event specification for those (e.g. no `events/in_angl0_*` > attributes). It sounds like this would be a problem as well? It's fine to have a channel that doesn't have controls or the ability to be read. We do have history of doing what you have here (a couple of accelerometers do it) but it's esoteric and rather hard for userspace to comprehend so I'd rather not introduce it for other types of devices. I think we should go with the most flexible option of allowing events to trigger when they 'may be true' to incorporate this case. Unfortunately I can't see another option that would scale to all the random combinations of events that might occur. There are all sorts of extensions we could make to the event descriptions, but only at the cost of breaking backwards compatibility and simplicity. SWith hindsight the whole IIO_MOD_X_OR_Y_OR_Z mess was a design error :( We can teach userspace code about that quirk for accelerations where the one that would be hard to handle is the AND case used for freefall detectors (you detect that the signal magnitude is near 0 for all axes). I can't think of another option for that one other than the weird modifier (unlike this case) > > Should we consider a IIO_EV_INFO_LABEL so that we can have some sort > of attribute (namely `events/<dir>_<channel spec>_label`) so that > userspace can enumerate expected events for non-configurable events? Probably needs something similar to channel labelling, so a separate callback given we don't handle strings, but sure something like this would be useful and provide 'hints' along the lines of what the datasheet calls a particular event. Not however for what event is sent as such info should be apparent from the event naming. Jonathan
