feat: implemented TranslateBrowsePathsToNodeIds service#309
Conversation
|
Thanks for contributing! Sorry for letting you wait. Holidays and higher priority product work delayed the review of your PR. We'll take a look. |
uklotzde
left a comment
uklotzde
left a comment
There was a problem hiding this comment.
Only some minor remarks from my side.
| impl TranslateBrowsePathsToNodeIdsResponse { | ||
| #[must_use] | ||
| pub fn results(&self) -> Option<ua::Array<ua::BrowsePathResult>> { | ||
| ua::Array::from_raw_parts(self.0.resultsSize, self.0.results) |
There was a problem hiding this comment.
Please add the TODO comments about returning "non-owned" results from similar methods. Just for consistency.
| /// # Errors | ||
| /// This fails only when the entire request fails. When a path does not exist or cannot be | ||
| /// translated, an inner `Err` is returned. | ||
| pub async fn translate_browse_path( |
There was a problem hiding this comment.
This is just a convenience method for very simple use cases.
We should strive to reduce the amount of redundant code by calling self.translate_many_browse_paths(). Even if it has a minor performance overhead.
See also: read_attributes()/read_many_attributes()
| return Err(Error::internal("unexpected number of browse path results")); | ||
| } | ||
|
|
||
| let targets: Vec<_> = results.iter().map(to_browse_path_result).collect(); |
There was a problem hiding this comment.
Let's use the turbofish variant here. See also: #312
| } | ||
| } | ||
|
|
||
| // translate many browse paths |
There was a problem hiding this comment.
We use sentence-style line comments for consistency and readability. Starting with a capital letter and ending with a period. Even if the comment is not a complete sentence.
|
We also need a changelog entry. |
2 participants
No description provided.