Community
Participate
Working Groups
As discussed in Bug 473064 Comment 32 it is proposed that a capsule part should have one field 'Replication' for setting the multiplicity of a capsule part. Thus the current field 'Multiplicity' should be renamed and its semantic needs to be adjusted. The rules for updating the 'Replication' field should be that if lower bound is zero, i.e. the case for optional and plugin capsule parts, then only upper bound should be set to the value of the 'Replication' field. If lower bound is not zero, then both lower bound and upper bound should be set to the same value according to the value entered in the 'Replication' field. It should be possible to both enter integer values as well as string values, and the relevant literals should be created for lower and upper bound.
Please also see the related Bug 479352 for a replication field for a port.
Please also see Bug 482923 related to the code-generator and the support for a replication factor in form of a string. Based on the outcome of this work related to the code-generator we might also need to support a "Browser..." button next to the replication field, where a constant (in form of an ownedAttribute of class, where that attribute have a default value) can easily be selected and then its qualified name can be automatically filled into the replication field (in the corresponding way as in the legacy tooling). It must also be settled whether an OpaqueExpression shall be used to specify the upper and lower bounds, or if a LiteralString shall be used. In legacy models an OpaqueExpression is being used, but the RSA import tool have functionality to convert all OpaqueExpressions to LiteralStrings (something that I doubt that we can force all legacy models to do during import/migration). If the string representation will support arithmetic operations (as described in Bug 482923) then I would actually prefer if the upper (and lower bound) where specified as OpaqueExpressions, to be able to have the language/body pairs and indicate that the expression actually is a kind of C++ expression.
New Gerrit change created: https://git.eclipse.org/r/62314
See also Bug Bug 482692: [Tooling] Optional Capsule should have hatching Pattern to update the code on that part.
New Gerrit change created: https://git.eclipse.org/r/71867
This gerrit propose this solution : 1) add a Replication filed (do not remove the Multiplicity one: keep it to show the impact of modifying the replication text on UML upper and lower multiplicity : this could be removed after validating the behavior of the newly added widget. 2) it is possible to both enter integer and string values (as proposed in the Description section) with some constraints as proposed in the ericsson forge and as I noticed in the RSA tool: 2.1: do not allow to enter a multiplicity x..y (just like in RSA tool : RSA propose a combobox and enable the string in format: x..y. this constraint was proposed by Peter in a comment on the ericsson forge 2.2: do not allow to enter real and negatif numbers (like -2.5, -5, 2.5 etc). I noticed this in the RSA tool when playing with the replication field (named Multiplicity) 2.3: allow the -1 and * values (that changes the upper value to * and the lower value to 0). this was suggested by Peter 3) The rules for updating the 'Replication' field: the rules specified in the Description of this bug are implemented: "... (1) if lower bound is zero, i.e. the case for optional and plugin capsule parts, then only upper bound should be set to the value of the 'Replication' field. (2) If lower bound is not zero, then both lower bound and upper bound should be set to the same value according to the value entered in the 'Replication' field..." 4) the rules for updating the lower/upper values: If the replication field is a positif Integer or equals to * or equals to -1, a LiteralInteger/UnlimitedNaturalInteger are created to specify the lower/upper value. otherwise 3) the rules specified in the Bug 479350 Description are implemented
4) the rules for updating the lower/upper values: 4.1 If the replication field is a positif Integer or equals to * or equals to -1, a LiteralInteger/UnlimitedNaturalInteger are created to specify the lower/upper value 4.2 If the replication filed value is a string (not a positif integer), an opaque expression is created with body= value 4.3 if the replication field value is an artihmetic expression, the C++ language is added to the opaque expression languages (the regex can be enhanced, it takes into account simple arithmetic expression like 5 + 2 -3, 5-x * 4+y, etc. without () 5) As suggested by Peter in Comment 2, a "Browser" button next to the replication field, where a constant (in form of an ownedAttribute of class, where that attribute have a default value) can easily be selected is added. The qualified name of the selected constant is automatically filled into the replication field (in the corresponding way as in the legacy tooling). we propose a filter in the displayed dialog to select the constant: only property with defaultvalue set could be selected. Asma
(In reply to smaoui asma from comment #6) > This gerrit propose this solution : > > 1) add a Replication filed (do not remove the Multiplicity one: keep it to > show the impact of modifying the replication text on UML upper and lower > multiplicity : this could be removed after validating the behavior of the > newly added widget. Remove the standard multiplicity field already now. If you want to validate the resulting multiplicity you can easily switch to the UML tab and check there. > > 2) it is possible to both enter integer and string values (as proposed in > the Description section) with some constraints as proposed in the ericsson > forge and as I noticed in the RSA tool: > > 2.1: do not allow to enter a multiplicity x..y (just like in RSA tool : > RSA propose a combobox and enable the string in format: x..y. this > constraint was proposed by Peter in a comment on the ericsson forge > > 2.2: do not allow to enter real and negatif numbers (like -2.5, -5, 2.5 > etc). I noticed this in the RSA tool when playing with the replication field > (named Multiplicity) Just to avoid any possible confusion: Please keep in mind that the multiplicity field in RSA is a true multiplicity field, which is what we want to get away from, and instead provide a single (and simplified) Replication field. (In reply to smaoui asma from comment #7) > 4) the rules for updating the lower/upper values: > > 4.1 If the replication field is a positif Integer or equals to * or equals > > to -1, a LiteralInteger/UnlimitedNaturalInteger are created to specify the > > lower/upper value > > 4.2 If the replication filed value is a string (not a positif integer), an > > opaque expression is created with body= value Has it been settled that it really should be an OpaqueExpression in all cases? As discussed it could be a LiteralString for the simple string case. If an OpaqueExpression is created in this case, which language is assigned to the body? There is currently an option in the import tool to convert OpaqueExpressions to LiteralStrings, which should be considered how it shall be used. I am not sure in which context it has been decided whether it shall be an OpaqueExpression or a LiteralString. So if this option is being used in the import tool, I guess models will have LiteralStrings instead (I have not tested this scenario in detail yet). This of course also have an impact on the code-generator which needs to be able to handle both the OpaqueExpression and the LiteralString cases. > > 4.3 if the replication field value is an artihmetic expression, the C++ > language is added to the opaque expression languages (the regex can be > enhanced, it takes into account simple arithmetic expression like 5 + 2 > -3, > 5-x * 4+y, etc. without () Is it "hard-coded" that the language assigned is C++ for this case, or does it use the default language framework to conclude which the default language is for the current model? > > 5) As suggested by Peter in Comment 2, a "Browser" button next to the > replication field, where a constant (in form of an ownedAttribute of class, > where that attribute have a default value) can easily be selected is added. > The qualified name of the selected constant is automatically filled into the > replication field (in the corresponding way as in the legacy tooling). > > we propose a filter in the displayed dialog to select the constant: only > property with defaultvalue set could be selected. Yes, we definitively need to filter the Browse dialog. And it must be aligned with the code-generator to support this case, i.e. Bug 482923. According to a comment Bug 482923 Comment 7, the support for constants will not be included in 1.0. Not sure how we shall handle this in 1.0, i.e. if the tooling should support entering constants, e.g. by browsing, but the code-generator simply rejects that case. Ernesto, do you have any opinion about this?
New Gerrit change created: https://git.eclipse.org/r/72000
(In reply to Eclipse Genie from comment #9) > New Gerrit change created: https://git.eclipse.org/r/72000 Was it really meant to create a new Gerrit change? I assume it was meant to be a new patch set for the existing Gerrit change, right?
(In reply to Peter Cigehn from comment #10) > (In reply to Eclipse Genie from comment #9) > > New Gerrit change created: https://git.eclipse.org/r/72000 > > Was it really meant to create a new Gerrit change? I assume it was meant to > be a new patch set for the existing Gerrit change, right? you are right it was a mistake, I forgot to click on amend when commiting my new changes, then when pushing , a new gerrit change is created. Please ignore this change, I upload a new patch set in the existing gerrit change. since I can not cancel the new gerrit change, I can not remove it :( Sorry for the mistake.
(In reply to smaoui asma from comment #11) > you are right it was a mistake, I forgot to click on amend when commiting my > new changes, then when pushing , a new gerrit change is created. > > Please ignore this change, I upload a new patch set in the existing gerrit > change. since I can not cancel the new gerrit change, I can not remove it : I assume that should be able to "Abandon" that Gerrit change that you created by mistake. At least to make sure that it does not "float around" in Gerrit with no use. You do have an "Abandon" button for that Gerrit change, right? I can remove that Gerrit change from being listed for this Bugzilla.
(In reply to Peter Cigehn from comment #12) > (In reply to smaoui asma from comment #11) > > you are right it was a mistake, I forgot to click on amend when commiting my > > new changes, then when pushing , a new gerrit change is created. > > > > Please ignore this change, I upload a new patch set in the existing gerrit > > change. since I can not cancel the new gerrit change, I can not remove it : > > I assume that should be able to "Abandon" that Gerrit change that you > created by mistake. At least to make sure that it does not "float around" in > Gerrit with no use. You do have an "Abandon" button for that Gerrit change, > right? I can remove that Gerrit change from being listed for this Bugzilla. Yes, I have the abandon button; I abandon the gerrit, but I wanted to cancel it (cancel the build (in hudson)) to avoid that it appears in bugzilla and thus no one is notified that I made a mistake :) however, I could not do this and the build is continued and you get the notification :(
> Has it been settled that it really should be an OpaqueExpression in all > cases? As discussed it could be a LiteralString for the simple string case. > If an OpaqueExpression is created in this case, which language is assigned > to the body? I followed the RSA tool that creates an opaque expression in all cases (no literal string) and did not assign a language to the body even for the arithmetic expression. if we prefer to differenciate 2 types of strings, we should specify what could be considered as simple string (to create literalString) and complex string (to create an opaque expression): > There is currently an option in the import tool to convert > OpaqueExpressions to LiteralStrings, which should be considered how it shall > be used. I am not sure in which context it has been decided whether it shall > be an OpaqueExpression or a LiteralString. So if this option is being used > in the import tool, I guess models will have LiteralStrings instead (I have > not tested this scenario in detail yet). I discussed with Camille, he told me that this was explicitly asked for him to transform opaque expresion to String, it was not his choice > Is it "hard-coded" that the language assigned is C++ for this case, or does > it use the default language framework to conclude which the default language > is for the current model? No iti is hard coded. OK to use the default language framework to return the language. > Yes, we definitively need to filter the Browse dialog. And it must be > aligned with the code-generator to support this case, i.e. Bug 482923. > According to a comment Bug 482923 Comment 7, the support for constants will > not be included in 1.0. Not sure how we shall handle this in 1.0, i.e. if > the tooling should support entering constants, e.g. by browsing, but the > code-generator simply rejects that case. Ernesto, do you have any opinion > about this? OK, I had in mind that the code generator supports the first case (constant in the model) and what is not in the scope of the 1.0 is only the 2 other cases where constants are not in the model (we did not support the 2 other cases as well). may be Ernesto could clarify this point to us).
Hi Peter, we did not conclude on the way the new replication field should handle the <undefined> multiplicities. actually this case is not handled, and as you point in the gerrit comments, this case should be handled: here is your suggestions: "...Keep in mind also the case where upper and lower value is not set at all, i.e. they semantically have the value of 1 according to the UML specification. I do know that the legacy tooling by default does not create any value specifications when you leave the default as one. The drop-down menu in the legacy tooling even have an explicit choice for "unsetting" the multiplicity, i.e. leave lower and upper value unset. In the legacy tooling this menu choice is name "None (1)" to indicate that the having no multiplicity equals 1. A lot of legacy models have multiplicity "unset" like this, and the new Replication field must be able to handle this case as well. So maybe adding a drop-down menu for the replication field with the choices of "None (1)", "1", and possibly "*" could be useful to handle this case.)..." I need some clarifications to handle this case: 1) In the current version, when we create a new capsulePart, the upper and the lower UML multiplicity are explicitly set to 1: we create the correspending valuespecification. did you prefer to change this behaviour and to leave the multiplicities unset when creating a new capsulePart? If yes, the default value of the replication field should be kept to "1" 2) If the user explicitly type "1" in the replication field, do you prefer to set the multiplicties upper and lower value to 1 or to unset those values ? In the RSA tool, if the user chose None(1) the multiplicities upper and lower values are unset, but the value of the replication field is reset to 1. so the "1" value in the replication field could have 2 semantics: either unset muliptlicties value or set multiplicities values =1 depending if the user chose "None(1)" or enter explicitly the value"1". I think that if the 2 cases did not really impact the semantic of the generated code, we should keep it as simple as possible. As the addition of the replication field aims to simplify the way the user modify the multiplicities (the user has just to modify one field and the corresponding valueSpecification will be created), the user should not be bothered by the actual lower and upper value specification. So, I do not really understand why the user needs to care about the UML multiplicities value specification: if they are unset or set to 1 (mainly if those 2 cases are semantically equivalent). I think that if there is no use to differentiate the 2 cases for the replication field values: - "None(1)" with unset upper and lower multiplicities - "1" with explicitly upper and lower multiplicities set to 1 we could keep the current simple implementation. however, if it is important to the user to have the ability to unset the mulitplicities values (upper and lower) from the replication field, then I agree that we should change the type of the field from String to Combobox and propose the None(1) value. and so, to not only propose one value, we could also propose "*" and "1" even though the user could enter those values directly (did not realy need a combobox for that). what do you think ? Thank you Asma
(In reply to smaoui asma from comment #14) > > Has it been settled that it really should be an OpaqueExpression in all > > cases? As discussed it could be a LiteralString for the simple string case. > > If an OpaqueExpression is created in this case, which language is assigned > > to the body? > > I followed the RSA tool that creates an opaque expression in all cases (no > literal string) and did not assign a language to the body even for the > arithmetic expression. if we prefer to differenciate 2 types of strings, we > should specify what could be considered as simple string (to create > literalString) and complex string (to create an opaque expression): For now I think that it is okay that you choose the OpaqueExpression to align with legacy. There is however one problem with this approach as originally reported in Bug 432912, which was the original cause of this functionality in the import tool. One other solution could of course to, in some way, ensure that those language-less OpaqueExpressions do get a defined language during import to avoid the issues with OCL evaluation on stuff that are not OCL. Since the root cause of this area of OpaqueExpression vs. LiteralString, I really think that we should avoid any mix, e.g. as discussed this about using LiteralString for "simple" strings and OpaqueExpressions for "complex" strings. As you say, then we need to define when to create what, which just makes things overly complex. We should avoid that. > > > There is currently an option in the import tool to convert > > OpaqueExpressions to LiteralStrings, which should be considered how it shall > > be used. I am not sure in which context it has been decided whether it shall > > be an OpaqueExpression or a LiteralString. So if this option is being used > > in the import tool, I guess models will have LiteralStrings instead (I have > > not tested this scenario in detail yet). > > > I discussed with Camille, he told me that this was explicitly asked for him > to transform opaque expresion to String, it was not his choice Yes, as can be seen in Bug 432912, the actual cause was that the OpaqueExpression lacks a language. There is not an issue with it being an OpaqueExpression as such. I think that we need to re-consider this functionality in the importer, and possibly make an RT specific solution, now when we also will have one RSA import in base-Papyrus, and one RSARTE specific one in Papyrus-RT. Then I assume that we shall be able to have RSARTE specific import behavior of these language-less OpaqueEpxression, e.g. to pick the language from the default language of the source model (which should be mapped over to the default language framework in Papyrus-RT). > > > > Is it "hard-coded" that the language assigned is C++ for this case, or does > > it use the default language framework to conclude which the default language > > is for the current model? > > No iti is hard coded. OK to use the default language framework to return > the language. This we must align with the import tool, and how it shall treat the default language in the source model. > > > > Yes, we definitively need to filter the Browse dialog. And it must be > > aligned with the code-generator to support this case, i.e. Bug 482923. > > According to a comment Bug 482923 Comment 7, the support for constants will > > not be included in 1.0. Not sure how we shall handle this in 1.0, i.e. if > > the tooling should support entering constants, e.g. by browsing, but the > > code-generator simply rejects that case. Ernesto, do you have any opinion > > about this? > > OK, I had in mind that the code generator supports the first case (constant > in the model) and what is not in the scope of the 1.0 is only the 2 other > cases where constants are not in the model (we did not support the 2 other > cases as well). may be Ernesto could clarify this point to us). Have Ernesto given any input regarding this yet? We need to settle what should be included in the tooling based on what support we will have in the code-generator in the 1.0 release. It will be a bit confusing if we have functionality in the UI which the code-generator will not support.
(In reply to smaoui asma from comment #15) > we did not conclude on the way the new replication field should handle the > <undefined> multiplicities. > > actually this case is not handled, and as you point in the gerrit comments, > this case should be handled: > here is your suggestions: > > "...Keep in mind also the case where upper and lower value is not set at > all, i.e. they semantically have the value of 1 according to the UML > specification. I do know that the legacy tooling by default does not create > any value specifications when you leave the default as one. The drop-down > menu in the legacy tooling even have an explicit choice for "unsetting" the > multiplicity, i.e. leave lower and upper value unset. In the legacy tooling > this menu choice is name "None (1)" to indicate that the having no > multiplicity equals 1. A lot of legacy models have multiplicity "unset" like > this, and the new Replication field must be able to handle this case as > well. So maybe adding a drop-down menu for the replication field with the > choices of "None (1)", "1", and possibly "*" could be useful to handle this > case.)..." > > I need some clarifications to handle this case: > > 1) In the current version, when we create a new capsulePart, the upper and > the lower UML multiplicity are explicitly set to 1: we create the > correspending valuespecification. did you prefer to change this behaviour > and to leave the multiplicities unset when creating a new capsulePart? If > yes, the default value of the replication field should be kept to "1" If we are going to align with legacy, then I guess we should leave them unset, since that is what the legacy tooling is doing. And this is also how legacy models being imported will look like (I assume that the import tool does not explicitly set lower and upper value in cases when they are left unset in the source model, I have not explicitly checked that since the import tool is not yet merged to the master branch for Neon). The same actually goes for ports, where the multiplicity is left unset by default also. I am not sure though what you really mean with 'the default value of the replication field should be kept to "1"'. In the legacy tooling, the label "None (1)" is displayed in this situation. If you see something else, maybe that is due to that you use an older version of the tool. > > 2) If the user explicitly type "1" in the replication field, do you prefer > to set the multiplicties upper and lower value to 1 or to unset those values > ? In the RSA tool, if the user chose None(1) the multiplicities upper and > lower values are unset, but the value of the replication field is reset to > 1. so the "1" value in the replication field could have 2 semantics: either > unset muliptlicties value or set multiplicities values =1 depending if the > user chose "None(1)" or enter explicitly the value"1". This is not the behavior I am seeing. If the user types "1" in the multiplicity field, then lower and upper value are explicitly set to 1. If the user selects "None (1)" from the combo-box drop-down menu, then lower and upper value are unset, and the label in the multiplicity field clearly indicates "None (1)" for this case. So there is not ambiguity between the unset case of 1 and the explicitly set case of 1 since the label is "None (1)" respectively "1". > > I think that if the 2 cases did not really impact the semantic of the > generated code, we should keep it as simple as possible. I agree that we shall try to keep it simple. But on the other hand, we also need to handle the import of legacy models in a defined way. The tooling must be able to handle imported legacy models, and also handle how imported legacy models are further evolved in Papyrus-RT. From an import perspective we need to decide if lower and upper value shall be imported "as is", i.e. if it is unset, be left unset. Or if we shall add new functionality to the import tool, which adjusts any unset lower and upper values, to ensure that the tooling (if we decide that it always expects lower and upper value to be set). > > As the addition of the replication field aims to simplify the way the user > modify the multiplicities (the user has just to modify one field and the > corresponding valueSpecification will be created), the user should not be > bothered by the actual lower and upper value specification. So, I do not > really understand why the user needs to care about the UML multiplicities > value specification: if they are unset or set to 1 (mainly if those 2 cases > are semantically equivalent). In principle I agree, if we didn't have to consider legacy models I would completely agree, and we had a "clean slate". > > > I think that if there is no use to differentiate the 2 cases for the > replication field values: > - "None(1)" with unset upper and lower multiplicities > - "1" with explicitly upper and lower multiplicities set to 1 > > we could keep the current simple implementation. What would the current tooling implementation do, if we import a legacy model "as is", with lower and upper value unset? Would it, without the user knowing, alter lower and upper value and explicitly set it? This would then be rather confusing for the user if unexpected changes occur in the model, e.g. when comparing with EMF Compare. Any changes from being unset to explicitly set, needs to be done consistently, to avoid that the user starts to see spurious changes, which the user haven't actually done. Sure, it might be that we need to have additional customization of EMF Compare to be able to handle the "Replication" field in itself (to avoid that the user is still confronted with lower and upper value in the UI of EMF Compare). Or would the tooling require that we alter the models already during import, to avoid that models never have lower and upper value left unset? I assume that the tooling needs to support, in some way or the other, that lower and upper value are unset (at least to ensure that it does not throw any exceptions) since models could be created by other means, e.g. programmatically or model transformations that create UML-RT models. > > however, if it is important to the user to have the ability to unset the > mulitplicities values (upper and lower) from the replication field, then I > agree that we should change the type of the field from String to Combobox > and propose the None(1) value. and so, to not only propose one value, we > could also propose "*" and "1" even though the user could enter those values > directly (did not realy need a combobox for that). > > what do you think ? If we decide to align with legacy, yes, then we should have a Combo-box, where the drop-down menu should have the option of "None (1)", "1" and "*". If "None (1)" is selected, and lower and upper value are unset, then the label "None (1)" should also be displayed to clearly indicate the difference between an explicitly set 1 vs. an unset 1. If we do not align with legacy, then we need to decide how imported legacy models shall be handled by the import tool and the tooling, and possibly write additional Bugzillas on the import tool for any additional cases that needs to be treated during import. If we cannot see some very, very strong argument why the unset case should not be support, I would prefer the alignment with legacy. Then at least we do know that we support the same kind of usage patterns that have been used in all kinds of legacy models.
(In reply to Peter Cigehn from comment #17) > (In reply to smaoui asma from comment #15) > If we are going to align with legacy, then I guess we should leave them > unset, since that is what the legacy tooling is doing. And this is also how > legacy models being imported will look like (I assume that the import tool > does not explicitly set lower and upper value in cases when they are left > unset in the source model, I have not explicitly checked that since the > import tool is not yet merged to the master branch for Neon). The same > actually goes for ports, where the multiplicity is left unset by default > also. > > I am not sure though what you really mean with 'the default value of the > replication field should be kept to "1"'. In the legacy tooling, the label > "None (1)" is displayed in this situation. If you see something else, maybe > that is due to that you use an older version of the tool. I am using IBM Rational® Software Architect RealTime Edition Version: 8.5. the same version (available here) was used by Camille to implement the importer. In this version, "None(1)" is not displayed, "1" is displayed even if the user chose "(none)". By the way you refer to "None(1)" and in this version, we have "(none)" > > 1. so the "1" value in the replication field could have 2 semantics: either > > unset muliptlicties value or set multiplicities values =1 depending if the > > user chose "None(1)" or enter explicitly the value"1". > > This is not the behavior I am seeing. If the user types "1" in the > multiplicity field, then lower and upper value are explicitly set to 1. If > the user selects "None (1)" from the combo-box drop-down menu, then lower > and upper value are unset, and the label in the multiplicity field clearly > indicates "None (1)" for this case. So there is not ambiguity between the > unset case of 1 and the explicitly set case of 1 since the label is "None > (1)" respectively "1". It is not the same behavior that I see in the 8.5 version. > I agree that we shall try to keep it simple. But on the other hand, we also > need to handle the import of legacy models in a defined way. The tooling > must be able to handle imported legacy models, and also handle how imported > legacy models are further evolved in Papyrus-RT. From an import perspective > we need to decide if lower and upper value shall be imported "as is", i.e. > if it is unset, be left unset. Or if we shall add new functionality to the > import tool, which adjusts any unset lower and upper values, to ensure that > the tooling (if we decide that it always expects lower and upper value to be > set). > currently, the importer did not modify the multiplicities, it keep them as defined in the RSA model. do you are right, we should manage the undefined case if in the legacy model contains undefined upper/lower. I think that we should not modify the importer, importing the multiplicities "as is" is the expected behavior of an importer tool. > What would the current tooling implementation do, if we import a legacy > model "as is", with lower and upper value unset? Would it, without the user > knowing, alter lower and upper value and explicitly set it? when I added the replication field, I have not in mind legacy models with undefined multiplicities, I mapped the replication field to the upper value of the multiplicity that is set to 1 for newly created capsule part. so, for me, unsetting the upper value is not allowed, since for newly created capsule the default value was 1. so actually, the tool did not do anything if we unset the upper value. the validator display the red cross and the field color becomes red because null <undefined> value is not supported. I am aware that I should modify this behavior, that's why I am wonderring if we really need to consider the undefined multiplicity value. now, and since we should consider the imported legacy models, the replication field should have a specific value that corresponds to the unset upper value : which could be "None(1)". I think then that even for the newly created capsulePart, we should not explicitly set the multiplicity to 1 (this is actually the case) because it is not the same behavior of the legacy tool: the multiplicty is set explicitly to "1" only if the user change it to "1" in RSA. do you agree ? Asma
(In reply to smaoui asma from comment #18) > I am using IBM Rational® Software Architect RealTime Edition Version: 8.5. > the same version (available here) was used by Camille to implement the > importer. In this version, "None(1)" is not displayed, "1" is displayed > even if the user chose "(none)". By the way you refer to "None(1)" and in > this version, we have "(none)" Well, that is a very old version (it came in 2012-2013), so that explains the difference. I am using a much never version (one of the absolutely latest deliveries, just a few weeks ago), so that is the behavior we shall try to follow. A lot of usability improvements have been made since that old 8.5 version, of which apparently this is one such area, e.g. to avoid the ambiguity between the unset and the explicitly set case that you pointed out. > > It is not the same behavior that I see in the 8.5 version. > No, but I would not expect it to necessary be the same since 8.5 is such an old version. > now, and since we should consider the imported legacy models, the > replication field should have a specific value that corresponds to the unset > upper value : which could be "None(1)". Keep the space as in the legacy, i.e. "None (1)". And include the case with "1" and possibly "*". We could possibly leave out the "*" in case the code-generator will not be able to handle it. The "*" case is probably only useful for non-code-generating models, and for those use-cases it might be sufficient if you only can type it (and not necessarily select it as pre-defined value in the drop-down menu of the combo-box). > > I think then that even for the newly created capsulePart, we should not > explicitly set the multiplicity to 1 (this is actually the case) because it > is not the same behavior of the legacy tool: the multiplicty is set > explicitly to "1" only if the user change it to "1" in RSA. do you agree ? > Yes, I agree. If we align with legacy behavior, and introduce the support for the unset case, then the default multiplicity should also be aligned, i.e. to leave it unset as default. And that should be aligned for both the capsule part case as well as the port case. We just needs to get clarification from the code-generator side, that the code-generator properly can handle the unset case. If the code-generator currently only use the derived property lower and upper, then the unset case will be handled automatically since those will be derived as 1 according to the UML specification. But I guess we need Ernesto to both confirm the aspect about which symbolic constant cases that the code-generator will support, as well as the support for the unset case of multiplicity for the capsule parts and ports.
(In reply to Peter Cigehn from comment #19) > But I guess we need Ernesto to both confirm the aspect about which symbolic > constant cases that the code-generator will support, as well as the support > for the unset case of multiplicity for the capsule parts and ports. We discussed what the code-generator will support in 1.0, and as already indicated in Bug 482923 Comment 7, the code-generator will not support symbolic constants in 1.0. So I suggest that we then at least wait with adding the "Browse..." button. The core support for OpaqueExpression (with an assigned language) we could probably include (even if the code-generator does not support it yet).
(In reply to Peter Cigehn from comment #20) > (In reply to Peter Cigehn from comment #19) > > But I guess we need Ernesto to both confirm the aspect about which symbolic > > constant cases that the code-generator will support, as well as the support > > for the unset case of multiplicity for the capsule parts and ports. > > We discussed what the code-generator will support in 1.0, and as already > indicated in Bug 482923 Comment 7, the code-generator will not support > symbolic constants in 1.0. So I suggest that we then at least wait with > adding the "Browse..." button. The core support for OpaqueExpression (with > an assigned language) we could probably include (even if the code-generator > does not support it yet). OK, and what about the the support for the unset case of multiplicity for the capsule parts and ports by the code generator ? Asma
(In reply to smaoui asma from comment #21) > > OK, and what about the the support for the unset case of multiplicity for > the capsule parts and ports by the code generator ? > I really think that we can add the unset case already now, so that we have the "core" functionality in place. If the code-generator have done this in the way I assume has been done, i.e. the code-generator probably uses the derived features 'lower' and 'upper', instead of reading a value specification from 'lowerValue' and 'upperValue', and then it will be able to handle the unset without really caring about it (since 'lower' and 'upper' returns 1 in the unset case). But I guess we should double-check with Ernesto. I guess we need to ping him explicitly since he does not seem to read the notifications from Bugzilla.
Sorry I'm late. If the value of the replication is a LiteralInteger, LiteralUnlimitedNatural or LiteralString, it will be used directly (details below *). So the user could enter anything in a LiteralString. I suppose OpaqueExpressions would work pretty much in the same way, and supporting them should be straightforward regarding attribute declarations. Basically, you could enter any C++ expression there, e.g. “1 + x * f(y)”, and the code generator would emit, e.g. “int attr[1 + x * f(y)]”. Of course, whether that complies or not depends on whether x, y and f are defined in that context. Now, the issue is that for attributes that is enough, but not so for ports and parts, because we need to know the exact value at generation time in order to generate the correct structure (connections and instances). At this point we are not doing anything about that. Basically, if the replication is left as a LiteralString, the structure generator fails with an exception. From Bug 482923 (https://bugs.eclipse.org/bugs/show_bug.cgi?id=482923) I quote that there were three cases: 1) Constants in the model 2) Constants in the transformation configuration file 3) Constants in an external .h-file (referenced by transformation configuration file) As we don’t have transformation configurations yet, and my understanding is that they are definitely not part of 1.0, that means that for 1.0 I won’t do anything about cases 2 and 3. As for case 1, it is a bit trickier than I thought. If the Browse button allows you to select an attribute, and we have a way of determining that it is an attribute rather than some arbitrary C++ expression, then I could create a patch to obtain the value of that attribute and use it as the replication. The problem is that I don’t see a simple way to determine whether the given string or OpaqueExpression is an attribute, that doesn’t involve traversing the model (expensive). Perhaps one could make the assumption that the attribute is local to the capsule, but that still requires traversal in the capsule for what I think should be a constant-time operation. On the other hand, if tooling was able to assign the ValueSpecification with some tag that identifies it as an attribute access, or make either a LiteralString or OpaqueExpression with a format like “attr:<name>” that can be parsed easily, it could become easier to handle. But I don’t like that too much as it relies on a very specific and ad hoc format. And then there is the ussue of whether we allow more complicated constant expressions, as described in https://bugs.eclipse.org/bugs/show_bug.cgi?id=482923#c2 of Bug 482923 (https://bugs.eclipse.org/bugs/show_bug.cgi?id=482923). So currently we don’t even support the basic constant case for ports and parts. So, is there a way to "mark" a value specification as a reference to an attribute? Or perhaps we should use OpaqueExpressions for attributes and LiteralStrings for anything else whose value cannot be determined? Or perhaps agree on a format like "attr:<name>"? As for the case where the multiplicity is unset, (set to null), this is supported, and in that case the default is 1 (see details below *). Some questions: I read a few comments in the bug: “allow the -1 and * values (that changes the upper value to * and the lower value to 0)” “If the replication field is a positif Integer or equals to * or equals to -1, a LiteralInteger/UnlimitedNaturalInteger are created to specify the lower/upper value” What are the values given if it is -1, and if it is *? How are these to be interpreted? What does it mean for a part to have replication -1 or *? If the “Browser” button is used and an attribute is selected and the attribute’s name is assigned, is it assigned as what? A LiteralString? Also, why are we considering the use of OpaqueExpressions? In UML an OpaqueExpression may have behaviour and parameters, but are we going to use these or is it just to align with the legacy? (*) Here’s how the code generator currently works: 1. Translation to the intermediate representation (xtumlrt): In xtumlrt any multiplicity element has a lower and upper bounds which are both of type String. So, during translation to xtumlrt it looks at the element’s ‘lowerBound’ and ‘upperBound’ which are UML ValueSpecifications. In both cases, if the value is null, it uses the default “1”. Otherwise, if it is a LiteralInteger or LiteralUnlimitedNatural, it gets its value and turns it into a String, assigns it to the xtumlrt lower/upper bound. If it is a LiteralString, it trims it and if it is not empty, assigns it to the xtumlrt lower/upper bound, or uses “1” if it is empty. For any other ValueSpecification, it assigns its ‘stringValue’ or “1” if empty. 2. Code generation from xtumlrt: 1. try to parse an int from the lowerBound string 2. try to parse an int from the upperBound string 3. if 1 was successful and and 2 was successful and lowerBound > upperBound, return lowerBound as the replication 4. if 1 was unsuccessful or lowerBound <= upperBound, return upperBound as the replication 5. if 1 was successful and 2 was unsuccessful, return the upperBound *string* as the replication 6. if 1 was unsuccessful and 2 was successful, return upperBound as the replication 7. if 1 was unsuccessful and 2 was unsuccessful, return the upperBound *string* as the replication When generating the relevant attribute declarations, we emit an array declaration with whichever string resulted from the algorithm above. But when generating the structure, if the computed bound is a string rather than an int, we raise an exception. We can adapt this to whatever the tooling does.
(In reply to Ernesto Posse from comment #23) > > From Bug 482923 (https://bugs.eclipse.org/bugs/show_bug.cgi?id=482923) I > quote that there were three cases: > > 1) Constants in the model > 2) Constants in the transformation configuration file > 3) Constants in an external .h-file (referenced by transformation > configuration file) > > As we don’t have transformation configurations yet, and my understanding is > that they are definitely not part of 1.0, that means that for 1.0 I won’t do > anything about cases 2 and 3. > > As for case 1, it is a bit trickier than I thought. If the Browse button > allows you to select an attribute, and we have a way of determining that it > is an attribute rather than some arbitrary C++ expression, then I could > create a patch to obtain the value of that attribute and use it as the > replication. Since the intention is that we shall align with legacy models, we should follow the principle used there. As indicated in Bug 482923 Comment 1 bullet 1, symbolic constants referenced in the model is done using its fully qualified name. The idea is that the "Browse..." button should allow the selection of a "constant" (attribute with a default value assigned), and then the tooling "fills in" the OpaqueExpression with the fully qualified name of that attribute. That is how the legacy tooling works, and how legacy models will look like. > The problem is that I don’t see a simple way to determine > whether the given string or OpaqueExpression is an attribute, that doesn’t > involve traversing the model (expensive). Perhaps one could make the > assumption that the attribute is local to the capsule, but that still > requires traversal in the capsule for what I think should be a constant-time > operation. Well, limiting the attribute to be owned by the capsule completely defeats the purpose of having a symbolic constant, since the usage is of course that the same symbolic constant should be possible to the reused as a replication factor among lots of capsule and their ports and capsule parts. Since the symbolic constant in this case always is a fully qualified name, I assume that the existence of "::" in the name should be sufficient to identify this case. > On the other hand, if tooling was able to assign the > ValueSpecification with some tag that identifies it as an attribute access, > or make either a LiteralString or OpaqueExpression with a format like > “attr:<name>” that can be parsed easily, it could become easier to handle. > But I don’t like that too much as it relies on a very specific and ad hoc > format. And then there is the ussue of whether we allow more complicated > constant expressions, as described in > https://bugs.eclipse.org/bugs/show_bug.cgi?id=482923#c2 of Bug 482923 > (https://bugs.eclipse.org/bugs/show_bug.cgi?id=482923). So currently we > don’t even support the basic constant case for ports and parts. So, is there > a way to "mark" a value specification as a reference to an attribute? Or > perhaps we should use OpaqueExpressions for attributes and LiteralStrings > for anything else whose value cannot be determined? Or perhaps agree on a > format like "attr:<name>"? Since legacy models are able to handle this with only using OpaqueExpression, really think that we should be able to do the same in Papyrus-RT. Keep it simple and only use OpaqueExpressions. To start with, we should be able to identify symbolic constants in the model based on the separator "::" in the fully qualified name. > > As for the case where the multiplicity is unset, (set to null), this is > supported, and in that case the default is 1 (see details below *). That was what I was assuming, and wanted to get confirmed. But then I suggest that the tooling aligns with legacy and should by default leave lowerValue and upperValue unset for both ports and capsule parts. > > > > Some questions: > > I read a few comments in the bug: > > “allow the -1 and * values (that changes the upper value to * and the > lower value to 0)” > > “If the replication field is a positif Integer or equals to * or equals > to -1, a LiteralInteger/UnlimitedNaturalInteger are created to specify the > > lower/upper value” > > What are the values given if it is -1, and if it is *? How are these to be > interpreted? What does it mean for a part to have replication -1 or *? Regarding the case of -1 and *, which both are exactly the same, i.e. lowerValue of LiteralInteger with value 0 and upperValue of LiteralUnlimitedNatural with value -1, we concluded that we should not support the -1 case, to make it easier and more consistent in the tooling and never allow negative values for the replication field. The remaining '** case is really only applicable to 'descriptive' models, and not 'prescriptive' models (if we use the terminology discussed on the mailing list), i.e. they are only usable in non-code generating models. In the first generation tooling (ObjecTime) it was possible to specify '*' as replication factor in a few cases (don't remember all the details), and if I remember correctly the code-generator was able to calculate the actual value itself (based on how things were connected) since the code-generator had knowledge about the complete model during generation time. But this "magic" was removed in the next generation tooling (RoseRT), since the code generator was generating code "per capsule", and thus could not conclude the actual value when using '*' as replication factor. I assume that we will exactly the same limitation also in the code-generator for Papyrus-RT (especially when building from the command line). Anyway, the support for the '*' case is only something that the tooling would support for the benefit of those doing 'descriptive' modeling. The code-generator could/should simply generate an error if it finds the '*' case, i.e. a LiteralUnlimitedNatural with the value -1. > > If the “Browser” button is used and an attribute is selected and the > attribute’s name is assigned, is it assigned as what? A LiteralString? No, as we have concluded we will align with legacy and let the "Browse..." functionality assign the fully qualified name of the selected attribute to an OpaqueExpression. LiteralString will not be used at all. > > Also, why are we considering the use of OpaqueExpressions? In UML an > OpaqueExpression may have behaviour and parameters, but are we going to use > these or is it just to align with the legacy? No, this is just a case of aligning with legacy and being able to import legacy models "as is" (what needs to be handled though is to update the import tool and let it assign a language to the OpaqueExpression, since the legacy models have "language-less" OpaqueExpressions which causes issues with OCL evaluation, see Bug 432912 for more about this. But instead of forcing OpaqueExpressions to become LiteralStrings as it is being done for RSA models, the current idea is to keep the OpaqueExpressions as is for RSARTE models, but instead assign the default language of the model to the OpaqueExpression during import to avoid the OCL issues with "language-less" OpaqueExpressions. Finally we then need to finally decide what to do with the "Browse..." button for the 1.0 release. As we discussed on the coordination meeting last Friday, I concluded that we should not support it. But, Ernesto, you indicate that it could still be possible to get it included in 1.0. Can you discuss this with Simon and Charles and you decide whether it is feasible for the code-generator to support the case with symbolic constant in the model, or if we simply leave it out of 1.0 (and thus also skip the "Browse..." button in the UI (which I already have indicated in Comment 20 and Bug 482923 Comment 8). I just want to get it confirmed that we do skip this for 1.0 or not.
(In reply to Peter Cigehn from comment #24) > Since the intention is that we shall align with legacy models, we should > follow the principle used there. As indicated in Bug 482923 Comment 1 bullet > 1, symbolic constants referenced in the model is done using its fully > qualified name. The idea is that the "Browse..." button should allow the > selection of a "constant" (attribute with a default value assigned), and > then the tooling "fills in" the OpaqueExpression with the fully qualified > name of that attribute. That is how the legacy tooling works, and how legacy > models will look like. Ah! I had forgotten about this. Can I assume then that if there is a '::' in the OpaqueBehaviour then it is case 1, and only in that case look up the value? The other bullet points (2-5) seem to be the cases we are not covering in 1.0, right? > > The problem is that I don’t see a simple way to determine > > whether the given string or OpaqueExpression is an attribute, that doesn’t > > involve traversing the model (expensive). Perhaps one could make the > > assumption that the attribute is local to the capsule, but that still > > requires traversal in the capsule for what I think should be a constant-time > > operation. > > Well, limiting the attribute to be owned by the capsule completely defeats > the purpose of having a symbolic constant, since the usage is of course that > the same symbolic constant should be possible to the reused as a replication > factor among lots of capsule and their ports and capsule parts. > > Since the symbolic constant in this case always is a fully qualified name, I > assume that the existence of "::" in the name should be sufficient to > identify this case. > [...] > Since legacy models are able to handle this with only using > OpaqueExpression, really think that we should be able to do the same in > Papyrus-RT. Keep it simple and only use OpaqueExpressions. To start with, we > should be able to identify symbolic constants in the model based on the > separator "::" in the fully qualified name. Indeed. I had forgotten about those rules. The presence of '::' should be enough. > > Some questions: > > > > I read a few comments in the bug: > > > > “allow the -1 and * values (that changes the upper value to * and the > > lower value to 0)” > > > > “If the replication field is a positif Integer or equals to * or equals > > to -1, a LiteralInteger/UnlimitedNaturalInteger are created to specify the > > > > lower/upper value” > > > > What are the values given if it is -1, and if it is *? How are these to be > > interpreted? What does it mean for a part to have replication -1 or *? > > Regarding the case of -1 and *, which both are exactly the same, i.e. > lowerValue of LiteralInteger with value 0 and upperValue of > LiteralUnlimitedNatural with value -1, we concluded that we should not > support the -1 case, to make it easier and more consistent in the tooling > and never allow negative values for the replication field. The remaining '** > case is really only applicable to 'descriptive' models, and not > 'prescriptive' models (if we use the terminology discussed on the mailing > list), i.e. they are only usable in non-code generating models. OK, so if the model happens to have replication '*' the user shouldn't be generating code, but what if he does? I suppose I should log an error message? > In the first generation tooling (ObjecTime) it was possible to specify '*' > as replication factor in a few cases (don't remember all the details), and > if I remember correctly the code-generator was able to calculate the actual > value itself (based on how things were connected) since the code-generator > had knowledge about the complete model during generation time. But this > "magic" was removed in the next generation tooling (RoseRT), since the code > generator was generating code "per capsule", and thus could not conclude the > actual value when using '*' as replication factor. I assume that we will > exactly the same limitation also in the code-generator for Papyrus-RT > (especially when building from the command line). Yes, we are generating code per capsule but we do have the model at hand (since capsules are defined inside models), and we could search the model for all uses of the capsule (not in opaque actions), although that would be an expensive operation, and it wouldn't work for capsules defined in one model but used in another. Furthermore, I imagine the rules could get complicated really quickly. > No, as we have concluded we will align with legacy and let the "Browse..." > functionality assign the fully qualified name of the selected attribute to > an OpaqueExpression. LiteralString will not be used at all. > Finally we then need to finally decide what to do with the "Browse..." > button for the 1.0 release. As we discussed on the coordination meeting last > Friday, I concluded that we should not support it. But, Ernesto, you > indicate that it could still be possible to get it included in 1.0. Can you > discuss this with Simon and Charles and you decide whether it is feasible > for the code-generator to support the case with symbolic constant in the > model, or if we simply leave it out of 1.0 (and thus also skip the > "Browse..." button in the UI (which I already have indicated in Comment 20 > and Bug 482923 Comment 8). I just want to get it confirmed that we do skip > this for 1.0 or not. I'll discuss with them. I think it is feasible. I'll open a bug for it but leave the target milestone unspecified for now.
Actually, should I open a new bug or leave it as part of Bug 482923? That bug covers all the cases. Perhaps a bug covering only the simpler case with qualified name could be preferable?
(In reply to Ernesto Posse from comment #25) > Ah! I had forgotten about this. Can I assume then that if there is a '::' in > the OpaqueBehaviour then it is case 1, and only in that case look up the > value? Yes, I assume so. But I guess that in the future, for the more complex cases where symbolic constants are defined by any of the external mechanisms that we decide to support, the existence of '::' should be determined per symbolic cases, i.e. you should be able to have arithmetic expressions based on a mix of symbolic constants in the model and external constants. But I guess that we must be able to assume that only a symbolic constant with '::' are located in the model. Other symbolic constants needs to be fetched externally. > > The other bullet points (2-5) seem to be the cases we are not covering in > 1.0, right? Yes, 2-5 are definitively out of 1.0. So the question is if 1 is also out of 1.0 release? > > OK, so if the model happens to have replication '*' the user shouldn't be > generating code, but what if he does? I suppose I should log an error > message? Yes, some error message shall be logged. In the future all errors/warnings from both the code-generator and the compilation, shall be presented in the problems view and be navigable back to the offending location in the model. I am not sure how much of this is included in 1.0, but I assume that some kind of error/warnings already can be generated from the code-generator, and this is just one more case. If symbolic constants are decided to be part of 1.0, then of course the error case where the referenced symbolic constant cannot be located, should also generate an error message. I assume that the code-generator must be able to detect all sorts of other error situations and should log similar error/warning messages (which preferably should end up in the problems views, with navigation support to get back to the offending location in the model). > Yes, we are generating code per capsule but we do have the model at hand > (since capsules are defined inside models), and we could search the model > for all uses of the capsule (not in opaque actions), although that would be > an expensive operation, and it wouldn't work for capsules defined in one > model but used in another. Furthermore, I imagine the rules could get > complicated really quickly. As I said, I don't see it feasible to calculate the useful cases in a real (large) model. The connector you refer to is only single assembly or delegation connectors as part of one capsule. The complex cases that ObjecTime could handle was a complete chain of delegation connectors, an assembly connector and then delegation connectors, passing up and down a bunch of relay ports. In the large case, you will not even have the model at hand, since a complete (aggregated) model will be stored in multiple (sub-)models. Considering these scaled up cases, I don't see it even possible that the code-generator shall be able to conclude the actual value of any '*' cases, and hence the legacy tooling stopped supporting this. Don't see any need of going down that path again...
(In reply to Ernesto Posse from comment #26) > Actually, should I open a new bug or leave it as part of Bug 482923? > > That bug covers all the cases. Perhaps a bug covering only the simpler case > with qualified name could be preferable? I think it is sufficient to keep in the scope of Bug 482923 (at least as long as we have decided to keep it out of 1.0, as already indicated in that Bugzilla). Only in the case that you really decide that we *shall* have it in 1.0 (which I doubt that we really need, but I think it is up to Charles to decided), then we should break it out into an own Bugzilla to track that case separately.
(In reply to Peter Cigehn from comment #28) [...Snip...] > I think it is sufficient to keep in the scope of Bug 482923 (at least as > long as we have decided to keep it out of 1.0, as already indicated in that > Bugzilla). Only in the case that you really decide that we *shall* have it > in 1.0 (which I doubt that we really need, but I think it is up to Charles > to decided), then we should break it out into an own Bugzilla to track that > case separately. This is not in scope for 1.0. BTW, feature freeze is this Friday (in line with Neon RC1). There is still more than enough work left to keep everyone busy on what we need to deliver at the end of June, so let not get distracted...
(In reply to Charles Rivet from comment #30) > (In reply to Peter Cigehn from comment #28) > [...Snip...] > > I think it is sufficient to keep in the scope of Bug 482923 (at least as > > long as we have decided to keep it out of 1.0, as already indicated in that > > Bugzilla). Only in the case that you really decide that we *shall* have it > > in 1.0 (which I doubt that we really need, but I think it is up to Charles > > to decided), then we should break it out into an own Bugzilla to track that > > case separately. > > This is not in scope for 1.0. > > BTW, feature freeze is this Friday (in line with Neon RC1). There is still > more than enough work left to keep everyone busy on what we need to deliver > at the end of June, so let not get distracted... OK. I'll leave it for later.
(In reply to Charles Rivet from comment #30) > (In reply to Peter Cigehn from comment #28) > [...Snip...] > > I think it is sufficient to keep in the scope of Bug 482923 (at least as > > long as we have decided to keep it out of 1.0, as already indicated in that > > Bugzilla). Only in the case that you really decide that we *shall* have it > > in 1.0 (which I doubt that we really need, but I think it is up to Charles > > to decided), then we should break it out into an own Bugzilla to track that > > case separately. > > This is not in scope for 1.0. > > BTW, feature freeze is this Friday (in line with Neon RC1). There is still > more than enough work left to keep everyone busy on what we need to deliver > at the end of June, so let not get distracted... +1
Hello, I have more questions on the support of the "unset multiplicities" to be handled by the replication field. We already concluded that the replication field should propose the "None (1)" default value: if the user choose this replication field value, the upper and the lower value specifications are unset (set to null). 1) what about if the user choose to set only the lower value (for example to 0) and keep the upper value unset ? what should be displayed in the replication field ? in my RSA version (an old version) the replication field display [0..1], but since our Replication Field is a simple String what should be the displayed value "None (1)" refers to unset both upper and lower value, so what would be the value of unset upper but set lower? 2) what about if the user choose to set only the lower value (for example to 5) and keep the upper value unset ? what should be displayed in the replication field ? in my RSA version (an old version) the replication field display [5..1], with optional kind which is not coherent but I suppose that Peter, with his latest version could end up with more coherent display. 2) what about if the user choose to set only the upper value (for example to 5) and keep the lower value unset ? what should be displayed in the replication field ? in my RSA version (an old version) the replication field display [1..5], with optional kind which is not coherent but I suppose that Peter, with his latest version could end up with more coherent display. Thanks Asma
(In reply to smaoui asma from comment #33) > I have more questions on the support of the "unset multiplicities" to be > handled by the replication field. > > We already concluded that the replication field should propose the "None > (1)" default value: if the user choose this replication field value, the > upper and the lower value specifications are unset (set to null). To be clear, they should be unset. I am not sure what the semantic is to set to null. We have had long discussion around the name property and leaving model elements unnamed by having the name property unset. This is something completely different from setting the name explicitly to null, since this causes the null value to be stored in the model file. So just to be very clear here: What we talk about is really to have lowerValue and upperValue unset, i.e. on Ecore level they should be considered to be unset. Not have it explicitly set to null value, which I assume is something completely different and will produce unnecessary information being persisted in the model file. > > 1) what about if the user choose to set only the lower value (for example to > 0) and keep the upper value unset ? what should be displayed in the > replication field ? in my RSA version (an old version) the replication field > display [0..1], but since our Replication Field is a simple String what > should be the displayed value "None (1)" refers to unset both upper and > lower value, so what would be the value of unset upper but set lower? I'm not fully sure that I understand the question. How do you mean that the user choose to set only the lower value? The purpose with the new replication field is that the user should not have to bother about the lower value. The way the user to "alter" the lower value in practice is by toggling the kind of capsule part, i.e. either fixed, optional or plugin, using the radio buttons. You cannot really compare this with RSARTE since in the legacy tooling the field is a "standard" multiplicity field, which explicitly shows, and allows edit of both lower and upper value. So there is no such thing as a "replication" field in RSARTE. That is what we want to make different in Papyrus-RT, and to simplify for the user, so that the user don't have to bother about the lower value. Keep in mind that the (new) replication field, never should show the .. notation, since that is only the notation used in a multiplicity field. But if we focus on your last part of your question, then yes, if the user selects "None (1)", then in practice both lower and upper value should be unset (not explicitly set to null, see my comment above), which in practice means that the capsule part will be a fixed capsule part with replication 1. If the user had selected the capsule part to be an optional or plugin capsule part, i.e. lower value is set to 0, then the capsule part will (when "None (1)" is selected) converted to fixed capsule part at the same time. This is exactly what is expected. The unset case is only applicable for fixed capsule parts. > > 2) what about if the user choose to set only the lower value (for example to > 5) and keep the upper value unset ? what should be displayed in the > replication field ? in my RSA version (an old version) the replication field > display [5..1], with optional kind which is not coherent but I suppose that > Peter, with his latest version could end up with more coherent display. I don't understand how you mean that the user can choose to set lower value and keep upper value unset? Sure if the user "fiddles" around with the advanced properties or programmatically creates such a model, then the tooling must have some well defined behavior. But as indicated above, you cannot compare with RSARTE and its multiplicity field and use of the .. notation, since that is not applicable to the replication field which basically only should display the upper value. We have at two constraints in the UML-RT profile to detect some anomalies in the setting of upper and lower value ("Fixed capsule parts cannot have variable multiplicity" and "Plugin capsule parts must have a lower multiplicity of zero", see Bug 489009 for more about this) and I think that should be sufficient. It should be enough if the replication field displays the upper value. > > 2) what about if the user choose to set only the upper value (for example to > 5) and keep the lower value unset ? what should be displayed in the > replication field ? in my RSA version (an old version) the replication field > display [1..5], with optional kind which is not coherent but I suppose that > Peter, with his latest version could end up with more coherent display. As indicated above, it should be enough if the replication field displays the upper value.
I don't know if I'm interpreting Asma's concerns well, but my guess is this: the RealTime tab should have only a "Replication" field whose value determines both the upper and lower bounds, however the UML tab still allows the user to modify these values separately. The question then is what should be the behaviour in that case?
(In reply to Peter Cigehn from comment #34) > (In reply to smaoui asma from comment #33) > To be clear, they should be unset. I am not sure what the semantic is to set > to null. We have had long discussion around the name property and leaving > model elements unnamed by having the name property unset. This is something > completely different from setting the name explicitly to null, since this > causes the null value to be stored in the model file. > > So just to be very clear here: What we talk about is really to have > lowerValue and upperValue unset, i.e. on Ecore level they should be > considered to be unset. Not have it explicitly set to null value, which I > assume is something completely different and will produce unnecessary > information being persisted in the model file. > Yes, I agree. > I'm not fully sure that I understand the question. How do you mean that the > user choose to set only the lower value? By going to UML tab and modifying the lowwer value (just like you are doing in Bug 484311 ) The user still have this possibility to modify UML mutiplicities directly from the UML Tab and I think that the Replication field should not display "None(1)" if the user already modify the lower value manually (i,e: with the UML TAB). > It should be enough if the replication field displays the upper value. This means that if the user did not modify the upper value (from the UML Tab), the Replication field still display "None(1)" even if the user modify the lower value and set it to 0 or any another integer. do you agree with such behavior ? Asma
(In reply to Ernesto Posse from comment #35) > I don't know if I'm interpreting Asma's concerns well, but my guess is this: > the RealTime tab should have only a "Replication" field whose value > determines both the upper and lower bounds, however the UML tab still allows > the user to modify these values separately. The question then is what should > be the behaviour in that case? yes, this was my question asked in smarter and more precise way ;)
(In reply to Ernesto Posse from comment #35) > I don't know if I'm interpreting Asma's concerns well, but my guess is this: > the RealTime tab should have only a "Replication" field whose value > determines both the upper and lower bounds, however the UML tab still allows > the user to modify these values separately. The question then is what should > be the behaviour in that case? Yes, if the user goes to the UML tab and alters multiplicity there, then the constraints defined in the UML-RT profile should (hopefully) pick any anomalies entered there, and we just need to find some reasonable behavior for the replication field on the RealTime tab. (In reply to smaoui asma from comment #36) > (In reply to Peter Cigehn from comment #34) > > (In reply to smaoui asma from comment #33) > > I'm not fully sure that I understand the question. How do you mean that the > > user choose to set only the lower value? > > By going to UML tab and modifying the lowwer value (just like you are doing > in Bug 484311 ) The user still have this possibility to modify UML > mutiplicities directly from the UML Tab and I think that the Replication > field should not display "None(1)" if the user already modify the lower > value manually (i,e: with the UML TAB). Yes, I agree. The label "None (1)" should *only* be displayed in the case that *both* lower and upper value are left unset. In all other cases, the value of upper value should be displayed, and this disregarding if upper value is set or not. In the case the upper value is not set, then its derived value is 1, and thus a "1" shall be displayed. > > > It should be enough if the replication field displays the upper value. > > This means that if the user did not modify the upper value (from the UML > Tab), the Replication field still display "None(1)" even if the user modify > the lower value and set it to 0 or any another integer. > > do you agree with such behavior ? No, as I indicated above, I think that it is more clear that the label "None (1)" should only be displayed if both upper and lower are unset. If "None (1)" is displayed in the "half unset case", then it is not clear to the user that one of lower or upper value actually is set.
I just checked in the legacy tooling, and the behavior is similar to as I indicated. The label "None (1)" is only displayed in the case where *both* lower value and upper value are left unset.
(In reply to Peter Cigehn from comment #38) > (In reply to smaoui asma from comment #36) > > This means that if the user did not modify the upper value (from the UML > > Tab), the Replication field still display "None(1)" even if the user modify > > the lower value and set it to 0 or any another integer. > > > > do you agree with such behavior ? > > No, as I indicated above, I think that it is more clear that the label "None > (1)" should only be displayed if both upper and lower are unset. If "None > (1)" is displayed in the "half unset case", then it is not clear to the user > that one of lower or upper value actually is set. Just to clarify the specific case of lower value being 0: If the user alters lower value specifically to 0, then in practice the capsule part will turn into an optional capsule part (or shared if also aggregation is changed to shared). And as I explained before, the "None (1)" case is *only* applicable to fixed capsule parts. As soon as you have an optional (or plugin) capsule part, the label "None (1)" is never applicable. And this goes hand in hand, with the ruling that "None (1)" only should be displayed if both lower and upper value are left unset.
Hello, I uploaded a new patch here: https://git.eclipse.org/r/#/c/71867/ that hopefully solves all discussed problems dealing with this new feature. I will try to propose an equivalent gerrit for Bug 479352 Asma
(In reply to smaoui asma from comment #41) > > I will try to propose an equivalent gerrit for Bug 479352 > When you look at Bug 479352, could you please also consider taking a look at Bug 484194 at the same time. When we also add the replication field to the port, the alignment between the order/layout of the properties view for ports and capsule parts becomes even more evident, and I think that this is good time to perform this alignment at the same time. Remi, can we assign Bug 484194 to Asma also, and include that together with the work with Bug 479352?
(In reply to Peter Cigehn from comment #42) > (In reply to smaoui asma from comment #41) > > > > I will try to propose an equivalent gerrit for Bug 479352 > > > > When you look at Bug 479352, could you please also consider taking a look at > Bug 484194 at the same time. When we also add the replication field to the > port, the alignment between the order/layout of the properties view for > ports and capsule parts becomes even more evident, and I think that this is > good time to perform this alignment at the same time. > > Remi, can we assign Bug 484194 to Asma also, and include that together with > the work with Bug 479352? Yes, I will assign the Bug 484194 to Asma.
This gerrit https://git.eclipse.org/r/73726 initially uploaded to fix Bug 479352 proposes also a factorization of the fix already uploaded for this bug in https://git.eclipse.org/r/71867. The factorization aim is mainly to not duplicate Editos/Dialogs/Properties/Observables for the Replication field added both to Port and Capsule Part.
Now when Bug 479352 has been fixed, I have also checked this one in the latest Papryus-RT build. The replication field for capsule parts looks good. It behaves as expected, i.e. for fixed capsule parts the lower and upper bound are the same, i.e. the value of the replication field. For optional and plug-in capsule parts, lower is always zero and upper is set to the value of the replication field. The only issue I could see was the slight confusion that when you have selected '*' from the drop-down list, in practice making the capsule part optional, then you cannot change the kind to fixed using the radio button. This kind of makes sense, but the user has not visual feedback, e.g. by disabling and making the "Fixed" radio button greyed out (compare with how the radio buttons for ports are disabled/greyed out for specific combinations). This is rather minor though, and the use of '*' for replication is only for non-code generating models, so this we can track in a separate Bugzilla if needed. I put this one inte resolved/verified fixed. Any additional issues can be tracked with separate Bugzillas.
Closing as verified fixed.
