On 12/08/2022 17:06, Arınç ÜNAL wrote: > > > On 12.08.2022 16:48, Krzysztof Kozlowski wrote: >> On 12/08/2022 16:41, Arınç ÜNAL wrote: >>> On 12.08.2022 10:01, Krzysztof Kozlowski wrote: >>>> On 12/08/2022 09:57, Krzysztof Kozlowski wrote: >>>>> On 12/08/2022 01:09, Arınç ÜNAL wrote: >>>>>>>> -patternProperties: >>>>>>>> - "^(ethernet-)?ports$": >>>>>>>> - type: object >>>>>>> >>>>>>> Actually four patches... >>>>>>> >>>>>>> I don't find this change explained in commit msg. What is more, it looks >>>>>>> incorrect. All properties and patternProperties should be explained in >>>>>>> top-level part. >>>>>>> >>>>>>> Defining such properties (with big piece of YAML) in each if:then: is no >>>>>>> readable. >>>>>> >>>>>> I can't figure out another way. I need to require certain properties for >>>>>> a compatible string AND certain enum/const for certain properties which >>>>>> are inside patternProperties for "^(ethernet-)?port@[0-9]+$" by reading >>>>>> the compatible string. >>>>> >>>>> requiring properties is not equal to defining them and nothing stops you >>>>> from defining all properties top-level and requiring them in >>>>> allOf:if:then:patternProperties. >>>>> >>>>> >>>>>> If I put allOf:if:then under patternProperties, I can't do the latter. >>>>> >>>>> You can. >>> >>> Am I supposed to do something like this: >>> >>> patternProperties: >>> "^(ethernet-)?ports$": >>> type: object >>> >>> patternProperties: >>> "^(ethernet-)?port@[0-9]+$": >>> type: object >>> description: Ethernet switch ports >>> >>> unevaluatedProperties: false >>> >>> properties: >>> reg: >>> description: >>> Port address described must be 5 or 6 for CPU port and >>> from 0 to 5 for user ports. >>> >>> allOf: >>> - $ref: dsa-port.yaml# >>> - if: >>> properties: >>> label: >>> items: >>> - const: cpu >>> then: >>> allOf: >>> - if: >>> properties: >> >> Not really, this is absolutely unreadable. >> >> Usually the way it is handled is: >> >> patternProperties: >> "^(ethernet-)?ports$": >> type: object >> >> patternProperties: >> "^(ethernet-)?port@[0-9]+$": >> type: object >> description: Ethernet switch ports >> unevaluatedProperties: false >> ... regular stuff follows >> >> allOf: >> - if: >> properties: >> compatible: >> ..... >> then: >> patternProperties: >> "^(ethernet-)?ports$": >> patternProperties: >> "^(ethernet-)?port@[0-9]+$": >> properties: >> reg: >> const: 5 >> >> >> I admit that it is still difficult to parse, which could justify >> splitting to separate schema. Anyway the point of my comment was to >> define all properties in top level, not in allOf. >> >> allOf should be used to constrain these properties. > > The problem is: > - only specific values of reg are allowed if label is cpu. > - only specific values of phy-mode are allowed if reg is 5 or 6. > > This forces me to define properties under allOf:if:then. None of the reasons above force you to define properties in some allOf:if:then subblock. These force you to constrain the properties in allOf:if:then, but not define. > Splitting to > separate schema (per compatible string?) wouldn't help in this case. True. > > I can split patternProperties to two sections, but I can't directly > define the reg property like you put above. Of course you can and original bindings were doing it. Let me ask specific questions (yes, no): 1. Are ethernet-ports and ethernet-port present in each variant? 2. Is dsa-port.yaml applicable to each variant? (looks like that - three compatibles, three all:if:then) 3. If reg appearing in each variant? 4. If above is true, if reg is maximum one item in each variant? Looking at your patch, I think answer is 4x yes, which means you can define them in one place and constrain in allOf:if:then, just like all other schemas, because this one is not different. Best regards, Krzysztof
