Bug 477811 - [Tooling] Combine the port and protocol message selection into one combined create trigger dialog
Summary: [Tooling] Combine the port and protocol message selection into one combined c...
Status: CLOSED FIXED
Alias: None
Product: Papyrus-rt
Classification: Modeling
Component: tool (show other bugs)
Version: 0.8.0   Edit
Hardware: PC Windows 7
: P3 normal
Target Milestone: 0.9.0   Edit
Assignee: Ansgar Radermacher CLA
QA Contact:
URL:
Whiteboard:
Keywords: Documentation
Depends on: 477721
Blocks: 509206 498289
  Show dependency tree
 
Reported: 2015-09-18 09:43 EDT by Peter Cigehn CLA
Modified: 2017-01-23 10:38 EST (History)
6 users (show)

See Also:


Attachments
UI mockup of a proposed create trigger dialog (19.05 KB, image/png)
2015-09-18 09:43 EDT, Peter Cigehn CLA
no flags Details
UI mockup of a proposed create port for trigger dialog (8.59 KB, image/png)
2015-10-02 08:19 EDT, Peter Cigehn CLA
no flags Details
Combined editor for port and trigger (70.77 KB, image/png)
2015-10-12 05:27 EDT, Ansgar Radermacher CLA
no flags Details
Combined editor for port and trigger (56.97 KB, image/png)
2015-11-27 05:19 EST, Ansgar Radermacher CLA
no flags Details
Screen shot from latest Papyrus-RT build (6.35 KB, image/png)
2015-11-27 08:54 EST, Peter Cigehn CLA
no flags Details
CreatePort dialog (21.47 KB, image/png)
2015-11-30 05:02 EST, Ansgar Radermacher CLA
no flags Details
Examples of legacy trigger dialog with ports of different types (55.70 KB, image/png)
2015-12-04 09:44 EST, Peter Cigehn CLA
no flags Details
Test model with multi-level protocol inheritance (6.40 KB, application/x-zip-compressed)
2017-01-13 06:33 EST, Peter Cigehn CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Cigehn CLA 2015-09-18 09:43:13 EDT
Created attachment 256674 [details]
UI mockup of a proposed create trigger dialog

This is a continuation of the work in Bug 471826 where the protocol message selection dialog was introduced. The next step to further streamline the creation of a transition trigger in a UML-RT state-machine is to combine the port selection and the protocol message selection into one common dialog.

Attached is a proposal how such a combined dialog could look like.

Some explanations:

* The list of ports shall be all behavior ports of the parent capsule (relay ports, i.e. non-behavior ports shall not be listed).
* Initially when no port has been selected the list of protocol messages is empty
* Selecting one port in the list of ports should fill the list of protocol messages with the relevant protocol messages depending on the conjugation of the port, i.e. it should work in the same way as in the current protocol message selection dialog
* If multiple ports are selected in the ports list, then the list of protocols messages should be the common subset of protocol messages between the protocols, i.e. the protocol messages from the common super class protocols, including the implicitly inherited from the base protocol with rtBound and rtUnbound, i.e. the UMLRTBaseCommProtocol from the UML-RT run-time model library (see Bug 477721 for a discussion of how the base protocol shall be identified). This is also already how it works in the current protocol message dialog (possibly an adjustment of how to locate the base protocol needs to be made depending on the outcome of Bug 477721).
* Whenever a port is deselected then of course the list of availble protocol messages shall be updated dynamically to reflect the current port selection.
* When selecting the * (AnyReceiveEvent) root node, then all the child nodes shall also be selected to indicate the semantic of selecting *. Deselecting any of the child nodes, should then cause the deselection of the * root node.
* If multiple protocol messages is selected, then at the time of pressing OK, then multiple triggers shall be created (since one trigger only can reference one event), where each trigger references the same set of selected ports. This can be an efficient way of creating multiple triggers based on multiple ports.
* The Create Port... and Create Protocol Message... should cause an additional dialog to popup providing the same functionality as when using the corresponding new child menu choices in the model explorer, e.g. creating a new port (on the parent capsule) should provide the possibility of also creating a new protocol. 
* The Create Protocol Message... button shall become disabled if multiple ports has been selected. When one port is selected the button will create a new protocol message in the protocol for the selected port. Depending on the conjugation either an in or out protocol message shall be created (it will not be possible to create a inOut protocol message this way). The newly created protocol message shall become automatically selected after creation.

Future improvements to this dialog could be include the possiblity of editing, or at least initiating the editing of any guard condition for the created trigger. But until we know more about how effect and guard code will be edited we leave that out for now.
Comment 1 Ansgar Radermacher CLA 2015-10-02 05:34:11 EDT
I'm currently working on the dialog and have a few questions.
(1) You state, message creation should only be enabled, if exactly one port is selected. Should we also enable it in case of multiple selected ports that use the same protocol? (Of course, the protocol must be writable)
(2) The "create port" button currently triggers directly a protocol selection dialog and uses a dummy name ("newPort"). Should the user be prompted for a port name first?
(3) How should the port/message information be presented in the protocol view while not editing it? Is it ok to present it like today, but with single "..." button for editing? (or two "...", but both targeting the same dialog?)
Comment 2 Peter Cigehn CLA 2015-10-02 08:00:35 EDT
(In reply to Ansgar Radermacher from comment #1)
> I'm currently working on the dialog and have a few questions.
> (1) You state, message creation should only be enabled, if exactly one port
> is selected. Should we also enable it in case of multiple selected ports
> that use the same protocol? (Of course, the protocol must be writable)

Yes, sure. If the two (or more) ports are typed by the same protocol, then the message creation button can be enabled as well. I just did not consider this since it feels like a rather rare scenario. But I checked the legacy tooling and this case is handled there as well. So the rule should be that if more than one ports is selected, then the create message button should be enabled as long as all selected ports are typed by the same protocol.

> (2) The "create port" button currently triggers directly a protocol
> selection dialog and uses a dummy name ("newPort"). Should the user be
> prompted for a port name first?

The create port button should preferably present a dialog where you can specify the name of the port, if the port is conjugated or not (the conjugation is so fundamental so this should be possible to define) and select whether to create a new protocol or select an existing protocol.

One option could be to present all the properties of the Real Time tab in the properties view for a port, that I feel that could be too cluttered. The use case when the user creates a port in this way only needs to very limited amount of available properties (as indicated above), and it is perfectly okay if the created port by default becomes an external behavior port.

If the user simply names the port and selects to create a new protocol, then the protocol should by default be given the same name as the port but with an initial capital letter.

If an existing protocol is selected, then the port can be automatically named (unless the user already has selected a name for the port) according to the selected protocol but with a initial small letter.

To assume that the protocol already exist will be too limiting for a use case where the user "builds up" the model via the trigger dialog.

> (3) How should the port/message information be presented in the protocol
> view while not editing it? Is it ok to present it like today, but with
> single "..." button for editing? (or two "...", but both targeting the same
> dialog?)

I am not sure I understand what you mean here. Do you really mean "in the protocol view"? Or do you mean in the properties view of the transition? In the properties view for the transition, a list of all the triggers should be presented with columns for the ports, protocol messages and guard.

If you mean the properties view for an individual trigger I am not sure. I actually expect the triggers to be edited from the table in the properties view of the transition where you can have a single edit button to bring up the same kind of dialog. The current "Real Time" tab for a trigger presents the information a bit strange, since you can add ports separately. I think that the port list and the protocol message should be combined into a single "property" (at least in the UI) with only one single edit button. This case however is not covered by the legacy tooling so I am not sure if we really need this. I think it will be more common to do the editing of the trigger(s) from the transition (if you have this table with multiple columns as I propose).
Comment 3 Peter Cigehn CLA 2015-10-02 08:19:29 EDT
Created attachment 256994 [details]
UI mockup of a proposed create port for trigger dialog

Here is simple mockup (mainly based on how it works in legacy tooling) for a dialog for creating a port for a trigger.
Comment 4 Ansgar Radermacher CLA 2015-10-12 05:27:35 EDT
Created attachment 257211 [details]
Combined editor for port and trigger

Snapshot of combined dialog as currently implemented on my machine. The screenshot also shows the sub-dialogs for port creation and protocol editing. Both reference existing dialogs. I propose to have a protocol editor instead of one creating a protocol message as the latter requires a bit more implementation effort (new dialog with additional combo-box for the direction). But no problem, if you think that the whole protocol editor is problematic (since you can remove protocol messages that are potentially being used).
Comment 5 Peter Cigehn CLA 2015-10-12 07:24:15 EDT
The combined dialog looks like a good start. Though the title of the dialog I would expect to at least mention that fact that you creating, or editing, a trigger. Right now the title does not really indicate its "trigger context". I would propose to name it "Create Trigger" as I proposed in the mockup, or possibly "Add Trigger" (as it was named in legacy tooling). For the case when the dialog is being re-used in the edit context, the title should be "Edit Trigger".

Regarding the use of existing dialog for creating ports and editing the protocol, I am not sure what to say. I think that we must understand the context for providing these buttons for creating a new port and creating a new protocol message directly from the create trigger dialog. 

In the normal case the user would probably have an existing model with ports and protocol messages. So then the trigger dialog is only about selecting what is already defined in the model. This model would then have been created the "normal" way, e.g. directly in the model explorer or in the capsule structure diagram.

But the use case when these two extra create buttons are being used, I would say, is in the quick prototype phase, when you build up a new model from scratch but are doings so from the context of a state machine. Another similar scenario will be from the context of a sequence diagram where the dialog for creating a message in a sequence diagram should have the similar ways of quickly creating new ports and protocol messages.

Anyway, in this scenario, when the user quickly wants to elaborate, prototype and build a completely new model from scratch, I think we should really focus on providing a simplified and more streamlined way of creating ports (and possibly new protocols) and protocol messages, than what is provided with the "full fledged" standard dialogs. It is for example not needed to provide all the detailed properties for creating a port in this scenario, and the proposed naming strategy, i.e. when the user gives the port a name, then the name of the protocol is derived automatically based on the naming rules for ports/protocols, is very important to avoid for the user to type the name of the protocol.

If the user is "confronted" with the (more complex) standard dialogs anyway, then I would say that the user can create the ports (and protocols) including protocol messages, prior to start creating triggers using the normal work flows.

I suggest that if we find it too complex and costly to create these kinds of simplified and more streamlined dialogs, then we skip the additional create buttons completely from this dialog, at least initially. 

Also I am not sure about the comment that the create protocol message dialog need to have a combo box for the direction. In this scenario the direction is given by the selected port (you only create a protocol message in the relevant direction that you currently are selecting protocol messages from). So there should not be needed for the user to make any such selection (I don't see that the user in this simplified work flow should be able to create inOut message, only 'in' or 'out' messages, depending on the conjugation of the port, is needed to be created).

Considering all this, I would also suggest that we break out the "Create port..." and "Create protocol messages..." buttons from this Bugzilla and write a separate Bugzilla for that future enhancement. Then we can plan that kind of streamlined improvement further. Possibly we can also combine the work with a future "Create message" dialog when editing UML-RT based sequence diagrams, since that customized dialog will need similar functionality for creating new ports and protocol messages. In legacy tooling it looks like the same kind of simplified streamlined dialogs are used in both contexts (state machine diagram and sequence diagram editing).
Comment 6 Eclipse Genie CLA 2015-11-27 05:05:47 EST
New Gerrit change created: https://git.eclipse.org/r/61430
Comment 7 Ansgar Radermacher CLA 2015-11-27 05:19:12 EST
Created attachment 258311 [details]
Combined editor for port and trigger

Modified dialogs. In the main dialog, the label for ports indicates conjugation by means of a back-slash prefix. The sub dialogs for creation are now simplified:
- ports: only conjugation station and protocol is shown. New port is automatically a behavioral port.
- protocol message: creation is uni-directional, direction depends on conjugation.
Comment 8 Peter Cigehn CLA 2015-11-27 07:46:12 EST
(In reply to Ansgar Radermacher from comment #7)
> Modified dialogs. In the main dialog, the label for ports indicates
> conjugation by means of a back-slash prefix. 

I am not sure I understand this prefix. The standard UML notation to indicate a conjugated ports is to put a ~ in front of the protocol name, i.e.

port : ~Protocol

Please check the UML 2.5 spec, section 11.3.4. page 189. Also check the model explorer or in a diagram, regarding how the name label for a conjugated port looks like. I know that the UML2 component was updated a while ago (not sure in which track it has been included yet, the commit was made 3 months ago at least), to ensure that the PortItemProvider returns a ~ in front of the port's type when the port is conjugated. I really don't see why we shall invent a new obscure notation for something that is already defined in the UML spec and provided by the UML2 component itself. I would also assume that this dialog should use the standard label provided by the UML2 component if it is already handled there.

> The sub dialogs for creation are now simplified:
> - ports: only conjugation station and protocol is shown. New port is
> automatically a behavioral port.
> - protocol message: creation is uni-directional, direction depends on
> conjugation.

This looks better than before. But I have a feeling that the creation of the protocol still can be further streamlined. In the legacy tooling it is sufficient to simply fill in a port name and then press the okay button for the case when you want to create a new protocol aslo. The protocol name is then deduced from the name of the port, i.e. the same name as the port but with the first character in upper case. 

From the looks of it here, you need to press the + button and then I assume that you current workflow with protocol creation, i.e. you have to select a container (which I have commented also for the standard port creation from the model explorer should be removed since it should be perfectly okay to place the protocol in the same container as the current capsule, see Bug 477821 Comment 0), and then I assume that you have to explicitly name the protocol. Then we still don't have gained that much by this simplified quick work flow to create a port from this dialog.

As indicated in the mockup I made for the port creation, I think it would be much better if you simply had a check box indicating if a protocol shall be created or not (which by default is checked). And in case it is checked, the protocol name is filled in automatically when you type in the port name. If you uncheck this, then the "Select..." button is enabled so that you can browse and select an existing protocol (which should be seen as the less common case).

As I indicated before. I we don't make this workflow as optimized as possible, then there is no or little use of having these buttons. Then the user equally well can create ports, protocols and protocol messages the ordinary work-flow. The main reason to have these buttons is to have an even faster more streamlined workflow, than the standard workflow.
Comment 9 Peter Cigehn CLA 2015-11-27 07:58:57 EST
Forgot to comment on the title of the dialog: Can't we decide whether is should be called "create" or "select"? 

If the intention is that the name shall cover both situations, i.e. both when you create a new trigger or edit/select another protocol message for an existing trigger, I would strongly suggest to have two separate titles for two use cases, to clearly indicate to the user what he/she is doing. Not having an "or" in the title, which could cause confusion to the user.

When the user adds/creates a new trigger, the title shall state only that, e.g. "Add Trigger" or "Create Trigger". And when the user edits and existing trigger, then the title shall state that, e.g. "Edit Trigger". Not sure if "Select Trigger" is good name, then "Reselect Trigger" would be more appropriate.

The wording should be aligned with how Papyrus names other similar operations.
Comment 10 Ansgar Radermacher CLA 2015-11-27 08:45:17 EST
(In reply to Peter Cigehn from comment #8)
> (In reply to Ansgar Radermacher from comment #7)
> > Modified dialogs. In the main dialog, the label for ports indicates
> > conjugation by means of a back-slash prefix. 
> ...
> 
> port : ~Protocol

Yes, this is the right notation. Whereas the ~ is shown in the diagram, it is  not in the model explorer (this is not only a Papyrus-RT, but a Papyrus issue in general - or even a UML2 issue). I talked to Remi, we will fix it globally (it was a bad idea to do something additional in this dialog).

> > The sub dialogs for creation are now simplified:
> > - ports: only conjugation station and protocol is shown. New port is
> > automatically a behavioral port.
> > - protocol message: creation is uni-directional, direction depends on
> > conjugation.
> 
> This looks better than before. But I have a feeling that the creation of the
> protocol still can be further streamlined. In the legacy tooling it is
> sufficient to simply fill in a port name and then press the okay button for
> the case when you want to create a new protocol aslo. The protocol name is
> then deduced from the name of the port, i.e. the same name as the port but
> with the first character in upper case. 
> [..]
> As indicated in the mockup I made for the port creation, I think it would be
> much better if you simply had a check box indicating if a protocol shall be
> created or not (which by default is checked). And in case it is checked, the
> protocol name is filled in automatically when you type in the port name. If
> you uncheck this, then the "Select..." button is enabled so that you can
> browse and select an existing protocol (which should be seen as the less
> common case).

From an implementation viewpoint, it would be simpler if not selecting an existing or creating a new protocol results in the automatic creating of a new protocol in the container of the capsule following the naming convention you describe. But we can also add the check-box.
[An issue in this context is that current naming convention for new ports proposes RTPort, i.e. a name starting with a capital]
Comment 11 Peter Cigehn CLA 2015-11-27 08:54:59 EST
Created attachment 258317 [details]
Screen shot from latest Papyrus-RT build

(In reply to Ansgar Radermacher from comment #10)
> (In reply to Peter Cigehn from comment #8)
> > (In reply to Ansgar Radermacher from comment #7)
> > > Modified dialogs. In the main dialog, the label for ports indicates
> > > conjugation by means of a back-slash prefix. 
> > ...
> > 
> > port : ~Protocol
> 
> Yes, this is the right notation. Whereas the ~ is shown in the diagram, it
> is  not in the model explorer (this is not only a Papyrus-RT, but a Papyrus
> issue in general - or even a UML2 issue). I talked to Remi, we will fix it
> globally (it was a bad idea to do something additional in this dialog).

Strange, I have the ~ notation shown on conjugated ports in the model explorer. Isn't the model explorer using the label providers directly from the UML2 component? For example, the model explorer do show keywords as supported by the UML2 component (which is not supported in diagrams in Papyrus). So I guess it depends on which version of UML2 that is used. And as I said, the correction in the UML2 component was made just a few months ago:

commit 0962276031e662e2452dc0054c8d1c04f4d7ad76
Author: Kenn Hussey <kenn.hussey@gmail.com> 2015-09-06 18:24:29
Committer: Kenn Hussey <kenn.hussey@gmail.com> 2015-09-06 18:24:29
Parent: 2653f1b68e4a644072ad3e07e38e399ad67b8f04 ([468342] Customizing label for start object behavior actions.)
Child: 9bb7cb613fe2c7bd19475fbde4e19b100a961659 (Adjusting RMAP for UML2 5.2 builds.)
Branches: master, origin/master

[476614] Adding tilde to label for conjugated ports.

Could it be related to this? Why am I seeing the ~ as in the screen shot?
Comment 12 Peter Cigehn CLA 2015-11-27 09:08:37 EST
(In reply to Ansgar Radermacher from comment #10)
> From an implementation viewpoint, it would be simpler if not selecting an
> existing or creating a new protocol results in the automatic creating of a
> new protocol in the container of the capsule following the naming convention
> you describe. But we can also add the check-box.
> [An issue in this context is that current naming convention for new ports
> proposes RTPort, i.e. a name starting with a capital]

As I have argued before: If we feel that it is complex from an implementation point of view, then skip these buttons completely. They should only be added to provide a simplified work-flow for the end-user. If that requires a too large implementation effort, then don't add them add at all. It will just a be a waste of effort to make "half baked" solutions that will not add any improvement to the end-user.

To make the creation of a new protocol as "hidden" as you describe is not very user-friendly and a little bit too "magic". We should really focus on the end-user experience here.

The proposed name when typing in the port name should just be a proposal (according to the naming rules). But the user should be able to adjust the proposed name prior to pressing the OK button, if needed. This is how it works in the legacy tooling and I find this workflow to be rather convenient.

Regarding the proposed naming, that seem to be a general issue. Also in other situations where ports are created, it does not follow the naming rule that ports should by default be named the same as the protocol but with a small initial letter, e.g. when using the tool on the palette in the capsule structure diagram or when using the UMLRealTime new child menu then you get a port named "portX" which is not named according to protocol either. 

It do work though with this naming scheme when you drag-and-drop a protocol to create a port. But even in this case the naming scheme need improvement since the port does not get a unique number in case of multiple ports is created based on the same protocol.
Comment 13 Ansgar Radermacher CLA 2015-11-27 10:04:06 EST
(In reply to Peter Cigehn from comment #11)
> Created attachment 258317 [details]
> Screen shot from latest Papyrus-RT build
> ...
> 
> Strange, I have the ~ notation shown on conjugated ports in the model
> explorer. Isn't the model explorer using the label providers directly from
> the UML2 component? For example, the model explorer do show keywords as
> supported by the UML2 component (which is not supported in diagrams in
> Papyrus). So I guess it depends on which version of UML2 that is used. And
> as I said, the correction in the UML2 component was made just a few months
> ago:
> 
> commit 0962276031e662e2452dc0054c8d1c04f4d7ad76
> Author: Kenn Hussey <kenn.hussey@gmail.com> 2015-09-06 18:24:29
> Committer: Kenn Hussey <kenn.hussey@gmail.com> 2015-09-06 18:24:29
> Parent: 2653f1b68e4a644072ad3e07e38e399ad67b8f04 ([468342] Customizing label
> for start object behavior actions.)
> Child: 9bb7cb613fe2c7bd19475fbde4e19b100a961659 (Adjusting RMAP for UML2 5.2
> builds.)
> Branches: master, origin/master
> 
> [476614] Adding tilde to label for conjugated ports.
> 
> Could it be related to this? Why am I seeing the ~ as in the screen shot?

I just verified with Remi that the "~" is correctly shown on his machine. Thus, it must be something related to my installation. After an update, I still have the issue with Mars and Neon installations within Papyrus, but the UML2 tree-editor works fine. UML version is from November 9th 2015. I´ll try to find out what is causing the problem.
Comment 14 Ansgar Radermacher CLA 2015-11-30 05:02:45 EST
Created attachment 258353 [details]
CreatePort dialog

Does this dialog look better? (besides changing the default name to a lower-caps name). The "create protocol" button is enabled by default and the text indicating the default protocol name in the following textbox is selected. This means that the user does not have to remove all characters of the existing name. In the moment, port and protocol name are not synchronized.
When the "Create protocol" button is not select, the textfield becomes gray and the user can use the standard dialog (grayed out before) that provides protocol selection or creation in a different container.
Comment 15 Peter Cigehn CLA 2015-11-30 08:23:30 EST
It is getting better. But I still think that having two fields for the protocol just feels very confusing. The same protocol field should be possible to use for both cases. The only thing that is needed is the ... button for selecting an existing protocol.  The other buttons regarding editing and deletion is not needed either.

I can understand that this is some default widget, but why use that in this context when it does not really fit with the context. I would assume that such a widget should be possible to configure to remove any unwanted buttons, like the create, edit and delete buttons, if they are not applicable.

I have feeling that we once again are a bit ahead of other Bugzillas that I would like to have resolved first. This default widget, with the buttons for selecting existing, creating new, edit existing or delete, is currently also used in other situations where I do not feel that it is fitting in either.

For example, see Bug 477821 which discusses the behavior of what I guess is the same widget. As can be seen there I really think that it is (in general) too complex to have to select a container for the protocol. I think that it is enough with by default simply create the protocol in the same container as the current capsule, i.e. both in the case when you create a port (based on a new protocol) from the new child menu in the model explorer, using the port tool on the palette in the capsule structure diagram, *including* this case where you create a new port and protocol when creating a new trigger in a state machine. In none of these cases, you should have to bother about selecting a container.

So the aspects of protocol creation and the use of this "standard widget" needs to be aligned and streamlined not only for this case, but also for the other two cases of port (and new protocol) creation from model explorer and capsule structure diagram as well. And if we have as default behavior for Papyrus-RT to always create additional elements like the protocol in this case in the the same container, then there is no need to explicitly spell that out in the dialog either (as has been done next to the first protocol field).

Regarding the names, the behavior should simply be that the port name and protocol name fields are empty to start with. When the user starts typing the port name, then the protocol name is filled in automatically based on the naming rules. So do not "pre-fill" a port name at all, just leave it empty. 

The name synchronization is one of the main feature of this dialog, for quickly creating a new port and protocol. And in this situation it is more probably more efficient to start with an empty port name (from which the protocol name is derived). Then when the port name has been filled in (and thus a proposed protocol name), the user can then go on and edit the protocol name if needed.
Comment 16 Peter Cigehn CLA 2015-12-04 03:25:55 EST
When looking into how system protocols should be identified (see Bug 477721) and how ports based on a system protocols should behave (see Bug 477033 Comment 27) I realised that we probably should remove the possibility to select rtBound/rtUnbound as triggers for the case of ports based on a system protocol.

Since it really does not make sense for a port based on a system protocol to generate notifications (and as indicated by Bug 477033 Comment 27) we shall prohibit the user from enabling notifications from SAPs based on system protocols, we should not include rtBound/rtUnbound in the list of available protocol messages for those ports. 

rtBound/rtUnbound shall only be available for selection for user-defined protocols.

This is also how it works in the legacy tooling.
Comment 17 Charles Rivet CLA 2015-12-04 09:22:18 EST
(In reply to Peter Cigehn from comment #16)
> When looking into how system protocols should be identified (see Bug 477721)
> and how ports based on a system protocols should behave (see Bug 477033
> Comment 27) I realised that we probably should remove the possibility to
> select rtBound/rtUnbound as triggers for the case of ports based on a system
> protocol.
> 
> Since it really does not make sense for a port based on a system protocol to
> generate notifications (and as indicated by Bug 477033 Comment 27) we shall
> prohibit the user from enabling notifications from SAPs based on system
> protocols, we should not include rtBound/rtUnbound in the list of available
> protocol messages for those ports. 
> 
> rtBound/rtUnbound shall only be available for selection for user-defined
> protocols.
> 
> This is also how it works in the legacy tooling.

Agreed.

In legacy apps, are rtBound/rtUnbound included in "*"?
Comment 18 Peter Cigehn CLA 2015-12-04 09:44:06 EST
Created attachment 258462 [details]
Examples of legacy trigger dialog with ports of different types

(In reply to Charles Rivet from comment #17)
> In legacy apps, are rtBound/rtUnbound included in "*"?

Yes, for user-defined protocols, rtBound/rtUnbound are included in the '*'. In the attachment I have showed that if you select the root '*', then the nested protocol messages automatically get selected, including rtBound/rtUnbound. So yes, they are included in '*'.

The rule that only the common subset of protocol messages shall be included in the list if multiple ports are selected, means that if the user both selects a port typed by a user defined protocol and a port typed by a system protocol, then basically it is only '*' that is the only common protocol messages. We discussed that rather extensively in the initial Bug 471826, but the discussion then mainly was for multiple user-defined protocols, in which case most often only '*' and rtBound/rtUnbound will be common.
Comment 19 Peter Cigehn CLA 2016-12-01 11:01:55 EST
I wrote Bug 508540 as a way of controlling the conjugation (and other properties) of a port in the port list in the dialog.
Comment 20 Peter Cigehn CLA 2016-12-02 04:28:42 EST
(In reply to Peter Cigehn from comment #19)
> I wrote Bug 508540 as a way of controlling the conjugation (and other
> properties) of a port in the port list in the dialog.

Considering that Bug 508540 was planned for Future (and I would really like to see that context menu for changing conjugation of the port within this trigger creation dialog), I suggest that we make a scope reduction for this bug. The support for adding new ports and protocol messages from within the trigger creation dialog we can add later, post-1.0 as a usability and work flow improvement. Then we can add the port properties context menu at the same time, instead of "forcing" in some other way of changing conjugation when creating new ports from within this dialog.

If you all agree with this, then I will write a separate bug for the tracking of adding (back) the "Create port..." and "Create protocol message..." buttons into this dialog.
Comment 21 Ansgar Radermacher CLA 2016-12-02 07:23:09 EST
(In reply to Peter Cigehn from comment #20)
> (In reply to Peter Cigehn from comment #19)
> > I wrote Bug 508540 as a way of controlling the conjugation (and other
> > properties) of a port in the port list in the dialog.
> 
> Considering that Bug 508540 was planned for Future (and I would really like
> to see that context menu for changing conjugation of the port within this
> trigger creation dialog), I suggest that we make a scope reduction for this
> bug. The support for adding new ports and protocol messages from within the
> trigger creation dialog we can add later, post-1.0 as a usability and work
> flow improvement. Then we can add the port properties context menu at the
> same time, instead of "forcing" in some other way of changing conjugation
> when creating new ports from within this dialog.
> 
> If you all agree with this, then I will write a separate bug for the
> tracking of adding (back) the "Create port..." and "Create protocol
> message..." buttons into this dialog.

I'd like to do some tests whether we could easily add a menu that you propose in 508540. If it's something that can be done with very small effort, I'd like to integrate this menu, otherwise I agree to remove these buttons.
Comment 22 Peter Cigehn CLA 2016-12-09 09:55:24 EST
(In reply to Peter Cigehn from comment #20)
> (In reply to Peter Cigehn from comment #19)
> > I wrote Bug 508540 as a way of controlling the conjugation (and other
> > properties) of a port in the port list in the dialog.
> 
> Considering that Bug 508540 was planned for Future (and I would really like
> to see that context menu for changing conjugation of the port within this
> trigger creation dialog), I suggest that we make a scope reduction for this
> bug. The support for adding new ports and protocol messages from within the
> trigger creation dialog we can add later, post-1.0 as a usability and work
> flow improvement. Then we can add the port properties context menu at the
> same time, instead of "forcing" in some other way of changing conjugation
> when creating new ports from within this dialog.
> 
> If you all agree with this, then I will write a separate bug for the
> tracking of adding (back) the "Create port..." and "Create protocol
> message..." buttons into this dialog.

As we decided on the synchronization meeting, we reduce the scope of this bug and break out the possibility of creating new ports (and protocols) and protocol messages into its own bug. I have written Bug 508994 to track this.

Maybe you can extract what has been implemented regarding this into its own Gerrit change and relate that to Bug 508994 so that we don't "loose" what you have done so far.
Comment 24 Eclipse Genie CLA 2017-01-13 04:21:30 EST
New Gerrit change created: https://git.eclipse.org/r/88620
Comment 25 Peter Cigehn CLA 2017-01-13 06:33:45 EST
Created attachment 266291 [details]
Test model with multi-level protocol inheritance

When testing the latest Gerrit change related to support for protocol inheritance, the attached model can be used for testing. It has an example with three level inheritance Base <- Sub <- SubSub, and the capsule has three ports based on each of these protocols. 

When selecting the port subSub it is expected that the protocol messages from both Base and Sub are included. The order should be: From system library base protocol (rtBound/rtUnbound), from Base, from Sub, from SubSub. This is also related to the properties view, see  Bug 507282 Comment 14. Preferably they should use the same functionality to retrieve and sort the list of protocol messages.
Comment 26 Eclipse Genie CLA 2017-01-17 06:48:16 EST
New Gerrit change created: https://git.eclipse.org/r/88850
Comment 27 Eclipse Genie CLA 2017-01-17 08:15:56 EST
New Gerrit change created: https://git.eclipse.org/r/88858
Comment 30 Peter Cigehn CLA 2017-01-19 07:09:40 EST
I have tested the latest Papyrus-RT build with the merged changes. The trigger create/edit dialog now works as expected, including the case with protocol inheritance and computing the common subset of protocol messages that can be received on all selected port in case of multiple ports being selected.

I suggest to put this one into resolved/verified fixed, whenever any missing unit tests have been.
Comment 31 Eclipse Genie CLA 2017-01-19 07:13:27 EST
New Gerrit change created: https://git.eclipse.org/r/89101
Comment 32 Eclipse Genie CLA 2017-01-19 10:27:34 EST
New Gerrit change created: https://git.eclipse.org/r/89117
Comment 34 Ansgar Radermacher CLA 2017-01-20 07:57:22 EST
Unit test has been added.
Comment 35 Peter Cigehn CLA 2017-01-20 09:19:57 EST
Verified to be fixed in the latest Papyrus-RT build. Remaining aspects regarding improved work-flow for creating new ports and protocol messages is tracked by Bug 508994.
Comment 36 Peter Cigehn CLA 2017-01-20 09:20:12 EST
Closing as verified fixed.
Comment 37 Ernesto Posse CLA 2017-01-20 17:04:35 EST
Sorry for reopening this Ansgar but I ran into a case that seems to have been missed: if a protocol message is typed with * then the message doesn't appear in the list of possible messages in the dialog.

See the UntypedProtocolMessage model in the git repo under models/tests/codepattern/structure.
Comment 38 Ernesto Posse CLA 2017-01-20 17:09:19 EST
Just to be clear: the message dialog does show a *, but that represents "any event". The issue in question is when there is a specific protocol message (called request in the sample model) which has a single parameter (data) whose type is * (accessible through the AnsiC library).
Comment 39 Peter Cigehn CLA 2017-01-20 18:10:46 EST
(In reply to Ernesto Posse from comment #37)
> Sorry for reopening this Ansgar but I ran into a case that seems to have
> been missed: if a protocol message is typed with * then the message doesn't
> appear in the list of possible messages in the dialog.
> 
> See the UntypedProtocolMessage model in the git repo under
> models/tests/codepattern/structure.

I checked this model, and from what I can see the problem is that the port has the wrong conjugation (or the protocol is defined in the wrong direction, dependending on how you view it).

Since the protocol does not have any in protocol messages (only an out protocol message), there are no protocol messages to select as triggers for an unconjugated port. If you switch the conjugation of the port, then the out protocol message will be available for selection, even if it is a protocol message with a parameter "typed" by *, i.e. untyped, which seem to be unrelated in this case.

If you agree with that this is the problem, then I suggest that we close this bug again.
Comment 40 Ernesto Posse CLA 2017-01-23 10:38:54 EST
Yes, you are right. It does work. My bad. Sorry. I'll close it.