Repository navigation
apply type checks in Blackboard::set when the key is remapped - #1232
Merged
facontidavide merged 1 commit intoOct 10, 2026
Merged
facontidavide merged 1 commit into
facontidavide merged 1 commit into
Conversation
This file contains hidden or 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
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.
Blackboard::set() looks the key up in the local storage only, so a key that a SubTree remapped into the parent blackboard never matches and the write always falls into the create-a-new-entry branch; createEntryImpl() follows the same remapping and returns the parent's existing entry, which is then overwritten with no type check at all. The result is that the same write behaves differently on the two sides of a subtree boundary: an unconvertible string written into an int entry through the remapping is stored as-is, leaving entry->info saying int while the value holds a std::string, a convertible string like "99" is kept as a raw string instead of being parsed to the declared type, and a safe int to uint8_t conversion that succeeds locally throws through the remapping. Every setOutput() on a remapped port takes this path, because the entry is created in the parent blackboard at build time and the subtree storage never holds the key. The workaround comment in set_blackboard_node.h ("avoid type issues when port is remapped") suggests callers have been compensating for this already. I ran into it while looking at the lookup asymmetries left after #1192. The fix resolves the entry with getEntry(), the same remapping-aware lookup every reader uses, so an existing entry goes through the type-checked branch and only a genuinely new key reaches createEntryImpl(); set() is an inline template in the header, so there is no ABI change. The regression test fails on master on all three behaviors and passes with the fix, and the full suite stays green.