aadl.library update #1
Conversation
proposed changed Ellidiss - 17 May 2024
There was a problem hiding this comment.
I left a few line-specific comments. Some describe changes that should be made before merging this Pull Request. Others are simply points for discussion and don't require changes.
I would want to see the following changes before merging:
- Don't remove the existing copyright notice from AADL.sysml
- Don't include an example in the library. Possibly move the example elsewhere or simply remove it from this Pull Request.
- Don't remove the
ActualBindingconnection defs from AADL.sysml - Add a comment to the
Actual_Processor_Bindingallocation def in Deployment_Properties.sysml
| @@ -1,26 +1,7 @@ | |||
| /** | |||
There was a problem hiding this comment.
Don't remove the existing copyright notice.
There was a problem hiding this comment.
Then, I will add mine too.
By principle, I cannot accept that our free contribution becomes copyrighted by another organization (knowing that we already agreed that it will be copyrighted by both SAE and OMG)
I would however prefer a copyleft approach for such a standardization collaborative work, otherwise it will soon become a complete mess.
A similar issue is the License file in the repository that is also irrelevant in this context.
| @@ -176,12 +163,6 @@ standard library package AADL { | |||
| connection def FeatureGroupConnection :> Connection; | |||
| connection def ParameterConnection :> Connection; | |||
|
|
|||
| abstract connection def ActualBinding; | |||
There was a problem hiding this comment.
Leave the existing ActualBinding connection defs in place. I know that you moved ActualProcessorBinding into Deployment_Properties and turned it into an allocation, but doing so breaks the translators.
After speaking with @jjhugues about this, we concluded that it would be best to leave both approaches in the library for now. We need to have further discussions about whether the AADL binding properties should be SysML connections or allocations before switching to allocations and updating the translators. I think that in the end, allocations are probably the way we will go, but we aren't there yet.
There was a problem hiding this comment.
There are three topics in your comment:
- what is the best mapping for bindings: I agree this still requires discussions. After the meeting in Reston, allocation seems to be the best candidate for now.
- location of the property definitions: I think we agreed that the best approach - for now - was to follow the structure of the AADL standard document. Binding properties are defined in appendix A1. The goal is to ease adoption of the SysML library by current AADL users and to ensure good visibility about the coverage of the mapping.
- several of us are developping tool prototypes. It is part of the job to update the prototypes with regards to changes in the common standard that we are elaborating together. It will not work if we constrain these changes to the current version of one of the tools.
| enum Background; | ||
| } | ||
|
|
||
| alias Periodic for Supported_Dispatch_Protocols::Periodic; |
There was a problem hiding this comment.
We should have a discussion about aliases for enumeration literals. I can see why you added them since enumeration literals in AADL are always referred to by a simple name and are never qualified. However, I feel that this could clutter the namespace. It might be a reasonable approach to use imports in the files that have values.
There was a problem hiding this comment.
Both approaches are acceptable for me.
It's more a question of modelling guidelines.
The real question is the status of the AADL_Project property set that was never well defined. To which extend is it actually customizable at an end-user or tool level ? Shouldn't we consolidate at least a common subset of these enumeration literals in the standard library. I believe we have enough feedback now to achieve that.
| @@ -0,0 +1,63 @@ | |||
| /* SysML v2 Domain Library for AADL: | |||
There was a problem hiding this comment.
This example is not a part of the library a should be excluded from this Pull Request. It might make sense to create a separate directory or even a separate repo for examples. Again, we should probably discuss where we can put examples.
There was a problem hiding this comment.
But we do need a way to store an exchange examples that are associated with a precise version of the library
Either we agree on some version numbering or we put library and examples in the same commits
Up to you.
|
|
||
| import AADL_Project::*; | ||
|
|
||
| allocation def Actual_Processor_Binding { |
There was a problem hiding this comment.
Please add a comment stating that this is a possible approach for handling AADL binding properties and that we need to have further discussions about it.
There was a problem hiding this comment.
No problem to add comments here and in other places too.
| @@ -1,4 +1,14 @@ | |||
| /* SysML v2 Domain Library for AADL: | |||
There was a problem hiding this comment.
We should also have a conversation about style. The changes to this file are style-only: file comments, import statement, and whitespace. What should the style be for the library?
There was a problem hiding this comment.
We started this discussion during the Reston meeting.
Maybe the best would be to align with other existing sysml standard libraries ?
proposed changed
Ellidiss - 17 May 2024