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

Move dtmi to public header #2342

Merged

Conversation

danewalton-msft
Copy link
Member

@danewalton-msft danewalton-msft commented Sep 16, 2022

The parsing APIs are developed to be hardcoded for a specific version of the DTMI. If the DTMI were to change, our underlying parsing would also have to change. I see no reason why we shouldn't make this public, since users can then reference it in their own published DTMI (and not have to create this string themselves).

For example, in the middleware sample, I don't see a reason why the user should have to provide this string. We should make this available to them to use.

https://github.com/Azure-Samples/iot-middleware-freertos-samples/blob/aa3d65a6e930062319bc4802d7cc1a00d0169570/demos/sample_azure_iot_adu/sample_azure_iot_adu.c#L216-L219

Copy link
Member

@CIPop CIPop left a comment

Choose a reason for hiding this comment

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

Left one comment but I don't have strong feelings either way.

@danewalton-msft
Copy link
Member Author

/check-enforcer evaluate

@jspaith
Copy link
Contributor

jspaith commented Sep 19, 2022

Just so I understand scenario, this is so that an application can pass in AZ_IOT_ADU_CLIENT_AGENT_INTERFACE_ID when registering an interface ModelId, correct? So it's a convenience to them to avoid them duplicating this since they'd need anyway?

In this case the change makes sense. And I think not having it be configurable is best since as you said customer cannot change this to like version2 and expect anything to work since there'd need to be .c changes.

@danewalton-msft
Copy link
Member Author

Just so I understand scenario, this is so that an application can pass in AZ_IOT_ADU_CLIENT_AGENT_INTERFACE_ID when registering an interface ModelId, correct? So it's a convenience to them to avoid them duplicating this since they'd need anyway?

In this case the change makes sense. And I think not having it be configurable is best since as you said customer cannot change this to like version2 and expect anything to work since there'd need to be .c changes.

Perfect yes this is exactly right. The DTMI is tied closely to how we parse the document. The only way customers would want this to change is if someone wants to extend it (or have it as a subcomponent in the future), in which case people are able to use this as a base or actually have in their DTMI itself. But the core of our parsing is still based on this value.

@danewalton-msft
Copy link
Member Author

/check-enforcer evaluate

@danewalton-msft
Copy link
Member Author

/azp run c - client - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@danewalton-msft danewalton-msft merged commit aabc319 into Azure:feature/iot-adu Sep 19, 2022
@danewalton-msft danewalton-msft deleted the dane/movedtmi branch September 19, 2022 16:12
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

Successfully merging this pull request may close these issues.

3 participants