Hey,
On Thu, Feb 08, 2024 at 07:24:58PM +0200, Alisa-Dariana Roman wrote:
> +patternProperties:
> + "^channel@([0-7a-f])$":
> + type: object
> + $ref: adc.yaml
> + unevaluatedProperties: false
> +
> + properties:
> + reg:
> + description: The channel index.
> + minimum: 0
> + maximum: 7
There are only 8 possible channels, at indices 0 to 7, so why is the
pattern property more permissive than that? Shouldn't "^channel@[0-7]$"
suffice?
> +
> + diff-channels:
> + description: |
> + The differential channel pair for Ad7194 configurable channels. The
> + first channel is the positive input, the second channel is the
> + negative input.
This duplicates the description in adc.yaml
> + items:
> + minimum: 1
> + maximum: 16
Hmm, this makes me wonder: why doesn't this match the number of channels
available and why is 0 not a valid channel for differential measurements?
Thanks,
Conor.
On 08.02.2024 20:03, Conor Dooley wrote:
> Hey,
>
> On Thu, Feb 08, 2024 at 07:24:58PM +0200, Alisa-Dariana Roman wrote:
>
>> +patternProperties:
>> + "^channel@([0-7a-f])$":
>> + type: object
>> + $ref: adc.yaml
>> + unevaluatedProperties: false
>> +
>> + properties:
>> + reg:
>> + description: The channel index.
>> + minimum: 0
>> + maximum: 7
>
> There are only 8 possible channels, at indices 0 to 7, so why is the
> pattern property more permissive than that? Shouldn't "^channel@[0-7]$"
> suffice?
>
>> +
>> + diff-channels:
>
>> + description: |
>> + The differential channel pair for Ad7194 configurable channels. The
>> + first channel is the positive input, the second channel is the
>> + negative input.
>
> This duplicates the description in adc.yaml
>
>> + items:
>> + minimum: 1
>> + maximum: 16
>
> Hmm, this makes me wonder: why doesn't this match the number of channels
> available and why is 0 not a valid channel for differential measurements?
>
> Thanks,
> Conor.
Hello and thank you for the feedback!
I will change the pattern property and the description.
Regarding the channels, I followed the existing style of the driver for
the AD7194 channels: one iio channel for each pseudo-differential input
channel(AINx - AINCOM), summing up to 16 channels; and one iio channel
for each differential channel (AINx - AINy), summing up to 8 channels.
For the diff-channels, I thought the possible values should be 1->16
corresponding to AIN1->AIN16 (I will add this to the description as
suggested by David).
Kind regards,
Alisa-Dariana Roman
On Thu, Feb 15, 2024 at 02:13:38PM +0200, Alisa-Dariana Roman wrote:
> On 08.02.2024 20:03, Conor Dooley wrote:
> > Hey,
> >
> > On Thu, Feb 08, 2024 at 07:24:58PM +0200, Alisa-Dariana Roman wrote:
> >
> > > +patternProperties:
> > > + "^channel@([0-7a-f])$":
> > > + type: object
> > > + $ref: adc.yaml
> > > + unevaluatedProperties: false
> > > +
> > > + properties:
> > > + reg:
> > > + description: The channel index.
> > > + minimum: 0
> > > + maximum: 7
> >
> > There are only 8 possible channels, at indices 0 to 7, so why is the
> > pattern property more permissive than that? Shouldn't "^channel@[0-7]$"
> > suffice?
> >
> > > +
> > > + diff-channels:
> >
> > > + description: |
> > > + The differential channel pair for Ad7194 configurable channels. The
> > > + first channel is the positive input, the second channel is the
> > > + negative input.
> >
> > This duplicates the description in adc.yaml
> >
> > > + items:
> > > + minimum: 1
> > > + maximum: 16
> >
> > Hmm, this makes me wonder: why doesn't this match the number of channels
> > available and why is 0 not a valid channel for differential measurements?
> >
> > Thanks,
> > Conor.
>
> Hello and thank you for the feedback!
>
> I will change the pattern property and the description.
>
> Regarding the channels, I followed the existing style of the driver for the
> AD7194 channels: one iio channel for each pseudo-differential input
> channel(AINx - AINCOM), summing up to 16 channels; and one iio channel for
> each differential channel (AINx - AINy), summing up to 8 channels.
I don't know what question this is answering. Everything here is about
channels so it is hard for me to relate it back.
Please reply inline & not at the end of the message to everything :)
Was it meant to answer the following?
> > > + properties:
> > > + reg:
> > > + description: The channel index.
> > > + minimum: 0
> > > + maximum: 7
> >
> > There are only 8 possible channels, at indices 0 to 7, so why is the
> > pattern property more permissive than that? Shouldn't "^channel@[0-7]$"
> > suffice?
If it was a response to this, the reg property only allows 8 channels so
the regex should only allow 8 too. The number after @ must match the number
in reg. If using each of the 16 "pseudo-differential" inputs in
isolation is thing you want to be able to do, your reg property does not
allow it.
> For the
> diff-channels, I thought the possible values should be 1->16 corresponding
> to AIN1->AIN16 (I will add this to the description as suggested by David).
With a description, this should be fine.