Offset calculation is multiplicative in GenICam + Default for increment in Integer Nodes #186
Conversation
|
I just noted that #182 exists. As correctly stated there, this PR does not fix all the problems but at least fixes one of them... |
frankjannis
changed the title
|
Hi! Something wrong with this PR or just currently too busy @Y-Nak :)? |
|
Hey @frankjannis, thanks for the PR, and sorry I missed this! The fix looks good, but it would be great to add a test that fails on Also, there seem to be two independent fixes here? It might be good to split them out to separate PRs if you don't mind, unless they're truly both required for the fix to make sense. We could also help to get the other fix in, related to using the length of the register when the offset is not specified. |
|
Hi @bschwind, thanks for responding! The 2 commits are indeed independent fixes and could be separated. |
That's okay, I might take a crack at it and see what I come up with.
It's true, but this crate is being used in several cameras running 24/7 capture and is quite stable, so there's not a lot of activity outside of bug fixes like this one. |
| @@ -45,7 +45,7 @@ impl Parse for IntegerNode { | |||
| let inc = node | |||
| .parse_if(INC, node_builder, value_builder, cache_builder) | |||
| .or_else(|| node.parse_if(P_INC, node_builder, value_builder, cache_builder)) | |||
| .unwrap_or(ImmOrPNode::Imm(10)); | |||
| .unwrap_or(ImmOrPNode::Imm(1)); | |||
There was a problem hiding this comment.
I believe this is correct, but is there somewhere in the spec which states this?
There was a problem hiding this comment.
I believe this is correct, but is there somewhere in the spec which states this?
https://www.emva.org/wp-content/uploads/GenICam_Standard_v2_1_1.pdf
Page 51. There are the defaults stated.
test_all.shis passed.fix #{ISSUE_NUMBER}if the corresponding issue exists.## Changelogsection. If the change is for only internal use, please writeNoneto the section.Changelog
Bugfix: The product of Offset and Index need to be added to the address for address offset calculations.
Bugfix: Default increment for Integer Nodes is 1.