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

Clarification on extending ValidateInstance #186

Open
jbradfordidx opened this issue Oct 3, 2024 · 5 comments
Open

Clarification on extending ValidateInstance #186

jbradfordidx opened this issue Oct 3, 2024 · 5 comments

Comments

@jbradfordidx
Copy link

As part of IoT Central, there's some additional property types that are supported within the dtmi:iotcentral:context;2 extension

      {
        "@id": "dtmi:sample:modelDefinition:HumidityThreshold;1",
        "@type": [
          "Property",
          "Cloud",
          "NumberValue"
        ],
        "displayName": {
          "en": "Humidity threshold"
        },
        "name": "HumidityThreshold",
        "schema": "double",
        "minValue": 0,
        "maxValue":  75
      }

Is there some additional sample code on adding support for custom properties that aren't natively supported within the parser? Unsure on if there's a way to decorate the schema validation based on additional types like NumberValue, Initialized, BooleanValue, StringValue, etc.

Currently, when I attempt to validate:

{
    "HumidityThreshold": 81
}

It is considered valid due to being a double value.

@jrdouceur
Copy link
Collaborator

I'm afraid we have no sample code for this use case. There is a tutorial on inspecting supplemental types and properties, which shows the approach you would follow to read the "maxValue" and "minValue" properties. The next step would be to add custom validation code that checks an instance value against these limits. At the moment, there is no automated generation of limit validation code, but it seems a worthwhile feature to consider.

@jbradfordidx
Copy link
Author

Thanks for your response. There doesn't appear to be support for extending the supplemental types, the API only provides it through read only collection or internal properties/classes.

For now, we have to check in the undefined properties collection and add our custom logic there to pull in data from the optional fields. One issue is that Azure IoT Central allows for DTMI elements that aren't supported in Azure Digital Twins resources. The durable property on commands for example will fail to parse if the command has been configured with the "Queue if offline" setting.

Are there any plans to be able to validate a DTInterfaceInfo? Was thinking I'd be able to pass in an entire Device Twin Desired Properties section to the DTInterfaceInfo.ValidateInstance method and it would validate all the properties/components/fields. If that support isn't desired, then perhaps a way for us to override that method would be helpful. Currently the constructors are all marked internal, so we can't derive from them.

@jrdouceur
Copy link
Collaborator

There doesn't appear to be support for extending the supplemental types, the API only provides it through read only collection or internal properties/classes.

Indeed, support for dynamically loading new supplemental types has been a common request, such as this one. (Adding this link reminds me that this is really a conversation about the parser, not about the DTDL language, so it is a bit more appropriate for the digitaltwinconsortium/DTDLParser repo than the Azure/opendigitaltwins-dtdl repo.)

Are there any plans to be able to validate a DTInterfaceInfo? Was thinking I'd be able to pass in an entire Device Twin Desired Properties section to the DTInterfaceInfo.ValidateInstance method and it would validate all the properties/components/fields.

No, this is not planned. DTDL does not define a representation for an Interface instance; this is defined by each service that implements DTDL, and it can vary across services.

If that support isn't desired, then perhaps a way for us to override that method would be helpful.

Would an extension method work for you?

@jbradfordidx
Copy link
Author

Thank you, you're right that this is related to the parser and not DTDL specifically. My apologies. Extension methods are an ok alternative. The only thing I might suggest is that if ValidateInstance will never be supported for certain classes, then to remove them from the class hierarchy for those as it can lead to confusion on what the library supports. If it can't be extended/overridden and will always throw an exception, I'm not sure I understand the reasoning for it to be there.

@jrdouceur
Copy link
Collaborator

Defining the abstract ValidateInstance method on the base class was a bit of an expedience. The parser is code-generated from the DTDL metamodel, and a concrete implementation of ValidateInstance is generated per the dtmm:instance property of any class that has this property (e.g., this one). There is no indication in the metamodel that there is anything special about the Schema class, so we would have to add some indication for the code generator to know that that this is the appropriate place to declare the abstract ValidateInstance method.

Since it would be a breaking change to remove ValidateInstance from the base class, we would have to evaluate the cost and impact of this breaking change relative to the benefit of placing the abstract method declaration at an arguably more appropriate place in the class hierarchy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants