Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to set keys on the global blackboard #527

Merged
merged 10 commits into from
Aug 16, 2024
Merged

Conversation

veerajsk
Copy link
Contributor

@veerajsk veerajsk commented Jul 31, 2024

  • The master plan can specify if its keys are meant for the global blackboard using the inherit_blackboard flag in the .plc file. If true, the keys are populated to the global blackboard & the master plan simply inherits the global blackboard as its own. If false, the behaviour remains unchanged.
  • Blackboard keys can now have default values

Design doc: https://gajanrapyutarobotics-my.sharepoint.com/:w:/g/personal/veeraj_khokale_rapyuta-robotics_com/EfxYaETn7ItNr5jGGRg4zjIBnXgWhDRmn5XFEhSS6Zr4EA?e=vkbIfr

  • Add unit test
  • Alica tests pass
  • Supplementary tests pass
  • Check turtlesim
  • Add support for default values
  • Add unit test for default values by regenerating etc folders using the new frontend

The master plan can specify if its keys are meant for the global
blackboard using the inherit_blackboard flag in the .plc file.
If true, the keys are populated to the global blackboard & the master
plan simply inherits the global blackboard as its own. If false,
the behaviour remains unchanged.
@veerajsk veerajsk added the Added New features or functionality label Jul 31, 2024
Copy link
Collaborator

@bjoernschroeder bjoernschroeder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

partial review, will continue tomorrow

int64_t masterKeyInBehavior = 0;
{
LockedBlackboardRW bb(*getBlackboard());
masterKeyInBehavior = bb.get<int64_t>("masterKey");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: std::int64_t

@bjoernschroeder
Copy link
Collaborator

PR is looking good to me so far. It would be great to have tests with different types defined in the global blackboard and using default values when the type is std::any.

@veerajsk veerajsk marked this pull request as ready for review August 16, 2024 10:38
@veerajsk
Copy link
Contributor Author

PR is looking good to me so far. It would be great to have tests with different types defined in the global blackboard and using default values when the type is std::any.

we can't have default values for std::any because we won't be able to parse it without knowing the type

Now that the frontend & backend work, I have setup a default key in testGlobalBlackbordMaster.pml & the unit test basically tests that global blackboard work as expected. I don't think we need a test for each type because the code is essentially the same as for value mapping & we already have many tests for that.

@veerajsk veerajsk merged commit 209b61e into devel Aug 16, 2024
4 checks passed
@veerajsk veerajsk deleted the vsk_gb_as_master_bb branch August 16, 2024 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Added New features or functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants