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

Allow using a UUID-based URN as the ID in the device's TD #6

Merged
merged 3 commits into from
Nov 24, 2024

Conversation

JKRhb
Copy link
Member

@JKRhb JKRhb commented Nov 24, 2024

At the moment, the device does not use a UUID as the id in its Thing Description but the device ID. As a potential improvement, this PR lets the application generate a UUID-based URN instead via the nuuid crate, which is probably a slightly better practice (although the device ID is probably also unique already). As this causes slightly more overhead, I am wondering whether this change should be hidden via a feature flag, with the old approach as a fallback.

Cargo.toml Outdated Show resolved Hide resolved
@lu-zero
Copy link
Contributor

lu-zero commented Nov 24, 2024

It looks good and adding a feature to switch back and forth might be a nice way to evaluate w3c/wot-thing-description#2054

@JKRhb JKRhb changed the title Use an UUID-based URN as the ID in the device's TD Allow using a UUID-based URN as the ID in the device's TD Nov 24, 2024
@JKRhb
Copy link
Member Author

JKRhb commented Nov 24, 2024

Okay, this PR should now be ready for a final review :)

@JKRhb JKRhb requested a review from lu-zero November 24, 2024 16:58
Copy link
Contributor

@lu-zero lu-zero left a comment

Choose a reason for hiding this comment

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

probably const-random-macro could be made optional, but can wait another iteration

@lu-zero lu-zero merged commit ae74b78 into wot-rust:master Nov 24, 2024
@JKRhb JKRhb deleted the uuid branch November 24, 2024 17:34
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.

2 participants