fix getDirectory for parameters #39
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mint-dewit
reviewed
Aug 5, 2024
| } | ||
|
|
||
| // this resulted in doubled subscription because getDirectory() adds the element as well |
There was a problem hiding this comment.
I don't understand what's going on here. Since the callback is never passed to the getDirectory call it should not cause a double subscription.
| cb, | ||
| }) | ||
| if (tree) { | ||
| const req = await this.getDirectory(tree) |
There was a problem hiding this comment.
The constant is never used so it is not necessary to assign it
Comment on lines
+397
to
+399
| const req = await this.getDirectory(tree) | ||
| tree = (await req.response) as NumberedTreeNode<EmberElement> | ||
| tree = getNextChild(tree, i) |
There was a problem hiding this comment.
Can you help me understand why this is needed in regards to the main issue this PR addresses?
The reason I am asking it makes it such that every time a library user uses this call the library will fetch all nodes in the path even though we have already fetched these before and can just look at them locally.
|
Closing as this is superseded by #40 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
About the Contributor
This pull request is posted on behalf of NEP Germany.
Type of Contribution
This is a: Bug fix
Current Behavior
In current releases of Lawo MC square devices you need to explicitly send a "Get Directory" request down to the parameter in order to get updates from the device. This is not the case. The "getElementByPath()" request currently only requests until the parent of the specified node.
New Behavior
"getElementByPath()" now explicitly requests the whole specified path. This fixes the problem, that sometimes no update is received from specific parameters
Status
[ x ] PR is ready to be reviewed.
[ x ] The functionality has been tested by the author.
[ x ] Relevant unit tests has been added / updated.
[ x ] Relevant documentation (code comments, system documentation) has been added / updated.