Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Support resolved URIs #95

Merged
merged 1 commit into from
Jan 18, 2024
Merged

Support resolved URIs #95

merged 1 commit into from
Jan 18, 2024

Conversation

stevenhartley
Copy link
Contributor

In order to support resolved UAuthority, we need to pass both name and IP and/or ID, otherwise we only have partial and isResolved() for remote URIs will never be true. The change is backwards compatible.

#94

In order to support resolved UAuthority, we need to pass both name and
IP and/or ID, otherwise we only have partial and isResolved() for remote
URIs will never be true. The change is backwards compatible.

 #94
Copy link
Contributor

@tamarafischer tamarafischer left a comment

Choose a reason for hiding this comment

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

This change does not contain a unit test. You said you are fixing a bug.
It would be a good idea to have a unit test with this PR showing that the bug is fixed

}
optional string name = 1; // Domain & device name as a string
optional bytes ip = 2; // IPv4 or IPv6 Address in byte format
optional bytes id = 3; // Unique ID for the device, could be a VIN, SHA 128, or any other identifier
Copy link
Contributor

Choose a reason for hiding this comment

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

if the id is a VIN, does that mean it does not appear in the name?
I think it is important to make this contract clear since a uE external to the vehicle usually uses the VIN in the datamodel to identify the car.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is vendor/OEM specific, we cannot dictate that in the protocol

@stevenhartley stevenhartley merged commit 642ed3f into main Jan 18, 2024
2 checks passed
@stevenhartley stevenhartley deleted the uauthority_fix branch January 18, 2024 15:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants