Bugzilla – Full Text Bug Listing
| Summary: | use LogicalThreads of SubSystemClass for thread deployment of actor instances (runtime) | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Modeling] eTrice | Reporter: | Henrik Rentz-Reichert <hrr> | ||||||
| Component: | Runtime | Assignee: | Thomas Jung <tj> | ||||||
| Status: | CLOSED FIXED | QA Contact: | |||||||
| Severity: | enhancement | ||||||||
| Priority: | P3 | CC: | ts | ||||||
| Version: | unspecified | ||||||||
| Target Milestone: | --- | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows 7 | ||||||||
| Whiteboard: | 0.1.0 | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 337076 | ||||||||
| Attachments: |
|
||||||||
|
Description
Henrik Rentz-Reichert
Created attachment 188896 [details]
enhamcement for Thread V1
enhancement for using threads. Sending messages by addressing directly the target message service. Cleaning up still necessary after decission if actor need to know its own message service. Patch contains runtime as well as templates.
Thanks for the patch! It looks already quite good. Please remove the TimingService.room from the patch (it looks like a spurious change introduced by the diagram editor). I would recommend to remove the connectAll() method from the MessageServiceController and do this in the addMsgSrv() method every time a message service is added. This ensures that all message services are connected with each other at any time. I would also like to suggest some minor changes - in the MessageDispatcher remove the two /*objectID*/ comments - in MessageServiceController use MessageService or the abbreviated form MsgSvc rather than MsgSrv (which could be understood as Message Server) - in InterfaceItemBase remove comment //ownMsgReceiver; - in SubSystemClassBase remove the comment // RTServices.getInstance().getMsgSrvCtrl... - in SubSystemClassBase name the method public MessageService getMsgService(int idx) consistent with the one in the MessageServiceController (adjust code generator after that change) Thanks for the comments. As i stated in earlier discussions the patch works just if the "connectAll" method is in place. As i figured out today, this is due to the fact that the TimingService accesses the MessageService in a user code section. After changing this it works fine. So, we need the TimingService within the patch. (In reply to comment #2) > Thanks for the patch! It looks already quite good. > Please remove the TimingService.room from the patch (it looks like a spurious > change introduced by the diagram editor). > I would recommend to remove the connectAll() method from the > MessageServiceController and do this in the addMsgSrv() method every time a > message service is added. This ensures that all message services are connected > with each other at any time. > I would also like to suggest some minor changes > - in the MessageDispatcher remove the two /*objectID*/ comments > - in MessageServiceController use MessageService or the abbreviated form MsgSvc > rather than MsgSrv (which could be understood as Message Server) > - in InterfaceItemBase remove comment //ownMsgReceiver; > - in SubSystemClassBase remove the comment // > RTServices.getInstance().getMsgSrvCtrl... > - in SubSystemClassBase name the method public MessageService getMsgService(int > idx) consistent with the one in the MessageServiceController (adjust code > generator after that change) Please fix also the the integration test.
In org.eclipse.etrice.integration.tests.IntegrationTestFSMGenerator
@Test(timeout=1000)
public void testHFSM(){
SubSystemHFSMTest main_component = new SubSystemHFSMTest(null,"MainComponent");
main_component.init(); // lifecycle init
main_component.start(); // lifecycle start
RTServices.getInstance().getMsgSrvCtrl().waitTerminate();
assertEquals(a_HFSM_Tester.STATE_TestPass ,main_component.getInstance("/MainComponent/application/HFSM_Tests/Tester").getState());
// end the lifecycle
main_component.stop(); // lifecycle stop
main_component.destroy(); // lifecycle destroy
}
In the room model (and similar in the .trp)
State TestPass {
entry {
"getMsgsvc().terminate();"
}
}
(In reply to comment #3) Of course if you remove connectAll() you have to do something like this: public void addMsgSrv(MessageService msgSrv){ // TODOTS: Who is parent of MessageServices ? if (msgSrv==null) return; // connect new message service with existing ones for (int i=0; i < messageServiceList.size(); i++) { MessageService ms = messageServiceList.get(i); ms.getMessageDispatcher().addMessageReceiver(msgSrv); msgSrv.getMessageDispatcher().addMessageReceiver(ms); } assert(msgSrv.getAddress().threadID == messageServiceList.size()); messageServiceList.add(msgSrv); } In the TimingService I don't see any substantial change. Do I miss the point? > Thanks for the comments. As i stated in earlier discussions the patch works just > if the "connectAll" method is in place. As i figured out today, this is due to > the fact that the TimingService accesses the MessageService in a user code > section. After changing this it works fine. So, we need the TimingService within > the patch. 1. changes in TimingService.room:
the changes must be made in the handler functions for the messages that shall be sent to the SAP(see below e.g. KILL). The message must be sent to the PeerMsgReceiver! (see below)
Did i something wrong??
conjugate PortClass {
usercode {
"private int currentId = 0;
private boolean active = false;"
}
handle timerTick {
"//conjugate PortClass handle timer
EventWithDataMessage dataMsg = (EventWithDataMessage) msg;
int id = (Integer)dataMsg.getData()[0];
if (active && id==currentId) {
getActor().receiveEvent(this, msg.getEvtId());
}"
}
handle Start {
"//conjugate PortClass handle start
if (active)
return;
active = true;
DebuggingService.getInstance().addMessageAsyncOut(getAddress(),
getPeerAddress(), messageStrings[IN_Start]);
=>>>>> getPeerMsgReceiver()
.receive(
new EventWithDataMessage(getPeerAddress(),
IN_Start, new TimerData(time_ms, ++currentId)));"
}
handle Kill {
"//conjugate PortClass kill
DebuggingService.getInstance().addMessageAsyncOut(getAddress(),
getPeerAddress(), messageStrings[IN_Kill]);
if (active) {
active = false;
=>>>>>>> getPeerMsgReceiver().receive(
new EventWithDataMessage(getPeerAddress(), IN_Kill, currentId));
}"
}
2. connectAll:
The goal is that the port which sends a message, directly addresses the MessageService from the receiving thread (no indirection via the own message service). The consequence is that the messageServices (one per thread) need not be connected to each other any longer. This connections just hides the errornous send method in the TimingService Protocoll.
Do you know what i mean ??? Is there any reason i didn´t see, to connect the messageServices ??
3. Fix the integrationtests:
I fixed the line in the IntegrationTestFSMGenerator, but i could not find the locations within the .room models.
(In reply to comment #5)
> (In reply to comment #3)
> Of course if you remove connectAll() you have to do something like this:
> public void addMsgSrv(MessageService msgSrv){
> // TODOTS: Who is parent of MessageServices ?
> if (msgSrv==null)
> return;
> // connect new message service with existing ones
> for (int i=0; i < messageServiceList.size(); i++) {
> MessageService ms = messageServiceList.get(i);
> ms.getMessageDispatcher().addMessageReceiver(msgSrv);
> msgSrv.getMessageDispatcher().addMessageReceiver(ms);
> }
> assert(msgSrv.getAddress().threadID == messageServiceList.size());
> messageServiceList.add(msgSrv);
> }
> In the TimingService I don't see any substantial change. Do I miss the point?
> > Thanks for the comments. As i stated in earlier discussions the patch works just
> > if the "connectAll" method is in place. As i figured out today, this is due to
> > the fact that the TimingService accesses the MessageService in a user code
> > section. After changing this it works fine. So, we need the TimingService within
> > the patch.
(In reply to comment #6) > 1. changes in TimingService.room: > the changes must be made in the handler functions for the messages that shall > be sent to the SAP(see below e.g. KILL). The message must be sent to the > PeerMsgReceiver! (see below) > Did i something wrong?? I know now what's our problem. The substantial changes already are contained in the eclipse.org repository ;-) Have you pulled the most recent changes? > 2. connectAll: > The goal is that the port which sends a message, directly addresses the > MessageService from the receiving thread (no indirection via the own message > service). The consequence is that the messageServices (one per thread) need not > be connected to each other any longer. This connections just hides the > errornous send method in the TimingService Protocoll. > Do you know what i mean ??? Is there any reason i didn´t see, to connect the > messageServices ?? Now I got the point. Following your reasoning it's not necessary to connect the message services at all. Thank you for pointing that out. > > 3. Fix the integrationtests: > I fixed the line in the IntegrationTestFSMGenerator, but i could not find the > locations within the .room models. line 249 of http://git.eclipse.org/c/etrice/org.eclipse.etrice.git/tree/tests/org.eclipse.etrice.integration.tests/model/org.eclipse.etrice.integration.tests.room Created attachment 189228 [details]
enhancement for Thread V2
Changes as discussed:
- remove connectAll()
- generate Addresses for Actor instances
- hand over the Address during instantiation instead of messageService
- remove hand over of message Service from the complete constructor hierarchy
Actor classes and Protocoll classes
- adapted the xpt templates and runtime files accordingly
Committed patch as http://git.eclipse.org/c/etrice/org.eclipse.etrice.git/commit/?id=c5e939ebca6576a089a4d3c8101f61fd7553371c and flagged it with iplog. Thanks for the contribution! Fixed integration test. tagged as version 0.1.0 |