Conversation
|
@dgl-cw thank you for your contribution. @RobertoRoos would you mind review the changes as you implemented the symbol handling? |
RobertoRoos
left a comment
RobertoRoos
left a comment
There was a problem hiding this comment.
I don't know about monitored variables, so I'm trusting this fix does it right.
|
|
||
| self.index_group = info.iGroup | ||
| self.index_offset = info.iOffs | ||
| if self.index_group == 0xF019: |
There was a problem hiding this comment.
Let's move this value to a constant alongside ADSIGRP_SYM_VALBYHND so the code is more readable.
There was a problem hiding this comment.
I'm not sure what to call this constant, as it is not documented, but I can make something up, if there are no guidelines for that case.
There was a problem hiding this comment.
I suggest something like ADSIGRP_SYM_MONITORDUMMY
There was a problem hiding this comment.
There should also be a comment to the group definition explaining that it's undocumented and that it's used in the context of monitored variables.
|
@dgl-cw did you manage to create some test cases for your suggested code? |
3 participants
Suggestion to fix #261