feat: add fine-grained policy merge #466
Open
+78
−10
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
Previously, if a parameter was added to the designer-provided policy, no automated policy determination would be performed. That meant that the init flag would not be computed in this case. To fix this, a merge_policy function was implemented to merge the explicit policy (the designer-provided one) and the computed policy (init, for instance) The fix follows these rules: - if a value is in policy, use it; - otherwise use the value from computed_policy Also, if the type policy is set to data, the size policy must be removed, as its only meaningful for the type buffer and it causes the connection to fail. The Ruby method `merge` merges two hashes. According to the documentation: "Returns a new hash containing the contents of other_hash and the contents of hsh. If no block is specified, the value for entries with duplicate keys will be that of other_hash." In this case, to follow the rules of prioritizing the value from explicit_policy in case of key duplication, 'other_hash' is explicit_policy and 'hsh' is computed_policy
8 tasks
jhonasiv
approved these changes
Feb 26, 2025
doudou
reviewed
Feb 27, 2025
There was a problem hiding this comment.
This is a good first step, but there's one issue.
When using needs_reliable_connection, we have to add period information to the syskit models. Until now, setting the policy explicitly would be enough to avoid having to do this (because of time, or because Syskit limitations would make it hard to do). This PR will break these cases.
My suggestion is to split policy_for into:
- policy_compute_data_element(policy, fallback_policy)
- policy_default_init_flag
Always call the latter, but call the former only if the explicit policy does not have :type set already. Create corresponding unit tests.
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.
Previously here: tidewise#3
This was on top of another
tools/syskitPR