Closed AaronGreenhouse closed 4 years ago
Further testing indicates the same problems for shared data connections, but that shared bus connections work as desired:
package WithDataConnections
public
subprogram ComputeAverage
end ComputeAverage;
data D
end D;
bus B
end B;
system s1
features
shared_bus: provides bus access B;
end s1;
system implementation s1.impl
subcomponents
myBus: bus B;
connections
bc1: bus access myBus <-> shared_bus;
end s1.impl;
system s2
features
ext_bus: requires bus access B;
end s2;
system implementation s2.impl
end s2.impl;
system MySystem
end MySystem;
system implementation MySystem.impl
subcomponents
myS1: system s1.impl;
myS2: system s2.impl;
connections
bc2: bus access myS1.shared_bus <-> myS2.ext_bus;
end MySystem.impl;
thread T1
features
shared_sub: provides subprogram access ComputeAverage;
shared_data: provides data access D;
end T1;
thread implementation T1.impl
subcomponents
CompAvg: subprogram ComputeAverage;
myData: data D;
connections
ac1: subprogram access CompAvg <-> shared_sub;
dc1: data access myData <-> shared_data;
end T1.impl;
thread T2
features
ext_sub: requires subprogram access ComputeAverage;
ext_data: requires data access D;
end T2;
thread implementation T2.impl
end T2.impl;
process p1
end p1;
process implementation p1.impl
subcomponents
MyT1: thread T1.impl;
MyT2: thread T2.impl;
connections
ac2: subprogram access MyT1.shared_sub <-> MyT2.ext_sub;
dc2: data access MyT1.shared_data <-> MyT2.ext_data;
end p1.impl;
system SubprogramExample19
end SubprogramExample19;
system implementation SubprogramExample19.impl
subcomponents
MyP: process p1.impl;
MyS: system MySystem.impl;
end SubprogramExample19.impl;
end WithDataConnections;
More tests:
subprogram
and data
)bus
)Complete test system:
package TestAccessConnections
public
subprogram ComputeAverage
end ComputeAverage;
subprogram group SubGroup
end SubGroup;
data D
end D;
bus B
end B;
virtual bus VB
end VB;
system s1
features
shared_bus: provides bus access B;
shared_VB: provides virtual bus access VB;
end s1;
system implementation s1.impl
subcomponents
myBus: bus B;
myVB: virtual bus VB;
connections
bc1: bus access myBus <-> shared_bus;
vbc1: virtual bus access myVB <-> shared_VB;
end s1.impl;
system s2
features
ext_bus: requires bus access B;
ext_VB: requires virtual bus access VB;
end s2;
system implementation s2.impl
end s2.impl;
system MySystem
end MySystem;
system implementation MySystem.impl
subcomponents
myS1: system s1.impl;
myS2: system s2.impl;
connections
bc2: bus access myS1.shared_bus <-> myS2.ext_bus;
vbc2: virtual bus access myS1.shared_vb <-> myS2.ext_vb;
end MySystem.impl;
thread T1
features
shared_sub: provides subprogram access ComputeAverage;
shared_subg: provides subprogram group access SubGroup;
shared_data: provides data access D;
end T1;
thread implementation T1.impl
subcomponents
CompAvg: subprogram ComputeAverage;
SubG: subprogram group SubGroup;
myData: data D;
connections
ac1: subprogram access CompAvg <-> shared_sub;
sgc1: subprogram group access SubG <-> shared_subg;
dc1: data access myData <-> shared_data;
end T1.impl;
thread T2
features
ext_sub: requires subprogram access ComputeAverage;
ext_subg: requires subprogram group access SubGroup;
ext_data: requires data access D;
end T2;
thread implementation T2.impl
end T2.impl;
process p1
end p1;
process implementation p1.impl
subcomponents
MyT1: thread T1.impl;
MyT2: thread T2.impl;
connections
ac2: subprogram access MyT1.shared_sub <-> MyT2.ext_sub;
sgc2: subprogram group access MyT1.shared_subg <-> MyT2.ext_subg;
dc2: data access MyT1.shared_data <-> MyT2.ext_data;
end p1.impl;
system SubprogramExample19
end SubprogramExample19;
system implementation SubprogramExample19.impl
subcomponents
MyP: process p1.impl;
MyS: system MySystem.impl;
end SubprogramExample19.impl;
end TestAccessConnections;
There are two main places in CreateConnectionsSwitch
that need to be update, and the correspond with the two different directions of the connection. I found these places pretty quickly by just searching for places where there are tests for BUS.
instantiateConnections()
there is the block of code if (hasOutgoingFeatureSubcomponents
&& ((cat != THREAD && cat != PROCESSOR && cat != DEVICE && cat != VIRTUAL_PROCESSOR)
// in case of a provides bus access we want to
// start from the bus.
|| ((cat == PROCESSOR || cat == DEVICE || cat == MEMORY)
&& feature instanceof BusAccess
&& ((BusAccess) feature).getKind() == AccessType.PROVIDES)) {
connectedInside = isConnectionEnd(insideSubConns, feature);
destinationFromInside = isDestination(insideSubConns, feature);
}
This clearly captures all the possibilities of provides bus access
features. (This is the case that currently works correctly.) So this needs to be expanded to capture the other provides acces
features.
appendSegment()
there is the block of code if (((toImpl instanceof ProcessorImplementation || toImpl instanceof DeviceImplementation
|| toImpl instanceof MemoryImplementation)
&& !(toEnd instanceof BusAccess
&& ((BusAccess) toEnd).getKind() == AccessType.PROVIDES))) {
final ConnectionInfo clone = connInfo.cloneInfo();
clone.complete = true;
finalizeConnectionInstance(ci, clone, toFi);
}
Again, this filters out the case of provides bus access
features. This also needs to be updated to handle the other provides access
features.
I started by trying to fix data access
connections. It is definitely true that these two locations must be updated, and I have done so as
if (hasOutgoingFeatureSubcomponents
&& ((cat != THREAD && cat != PROCESSOR && cat != DEVICE && cat != VIRTUAL_PROCESSOR)
// in case of a provides bus access we want to
// start from the bus.
|| ((cat == PROCESSOR || cat == DEVICE || cat == MEMORY)
&& feature instanceof BusAccess
&& ((BusAccess) feature).getKind() == AccessType.PROVIDES)
// in case of a provides data access we want to
// start from the data.
|| ((cat == DATA || cat == THREAD || cat == THREAD_GROUP || cat == PROCESS)
&& feature instanceof DataAccess
&& ((DataAccess) feature).getKind() == AccessType.PROVIDES))) {
connectedInside = isConnectionEnd(insideSubConns, feature);
destinationFromInside = isDestination(insideSubConns, feature);
}
and
if (((toImpl instanceof ProcessorImplementation || toImpl instanceof DeviceImplementation
|| toImpl instanceof MemoryImplementation)
&& !(toEnd instanceof BusAccess
&& ((BusAccess) toEnd).getKind() == AccessType.PROVIDES))
|| ((toImpl instanceof DataImplementation || toImpl instanceof ThreadImplementation
|| toImpl instanceof ThreadGroup || toImpl instanceof ProcessImplementation)
&& !(toEnd instanceof DataAccess
&& ((DataAccess) toEnd).getKind() == AccessType.PROVIDES))) {
final ConnectionInfo clone = connInfo.cloneInfo();
clone.complete = true;
finalizeConnectionInstance(ci, clone, toFi);
}
but this is not sufficient because the second block of code is guarded by an extremely complicated chain of conditions. Towards the start of this chain, there is test of the local variable finalComponent
which gets its value from isConnectionEndingComponent(toCtx)
, which is true
if the test component is a thread or device subcomponent. This has the effect of preventing the connection processing from going back "in" a thread component to find the nested data (or other subcomponent) being provided, event though this connection is subcomponent is connected and allowed to go "out" of the thread when the connection instance is generated in the other direction.
There is a test early on in this conditional chain
if (toEnd instanceof Parameter
|| finalComponent && !(toEnd instanceof FeatureGroup)) {
If I change it to
if (toEnd instanceof Parameter
|| finalComponent && !(toEnd instanceof FeatureGroup || toEnd instanceof Access)) {
Then data access connections work as required. As I have used the super type Access
this test should be sufficient for subprogram and subprogram group accesses as well. However, I am concerned this change could cause unexpected problems in other cases because I don't at all understand everything happening in this method because it is so extremely complicated. I need to keep an eye on the unit tests.
The logic for checking the end of the connection instance in appendSegment
got really tricky. I had to change it to
boolean finalizeConnectionInstance = true;
if (toEnd instanceof BusAccess && ((BusAccess) toEnd).getKind() == AccessType.PROVIDES) {
if (toImpl instanceof ProcessorImplementation || toImpl instanceof MemoryImplementation
|| toImpl instanceof BusImplementation || toImpl instanceof DeviceImplementation
|| toImpl instanceof SystemImplementation) {
finalizeConnectionInstance = false;
}
} else if (toEnd instanceof DataAccess
&& ((DataAccess) toEnd).getKind() == AccessType.PROVIDES) {
if (toImpl instanceof DataImplementation || toImpl instanceof ThreadImplementation
|| toImpl instanceof ThreadGroupImplementation
|| toImpl instanceof ProcessImplementation
|| toImpl instanceof SystemImplementation) {
finalizeConnectionInstance = false;
}
} else if (toEnd instanceof SubprogramAccess
&& ((SubprogramAccess) toEnd).getKind() == AccessType.PROVIDES) {
if (toImpl instanceof DataImplementation
|| toImpl instanceof SubprogramGroupImplementation
|| toImpl instanceof ThreadImplementation
|| toImpl instanceof ThreadGroupImplementation
|| toImpl instanceof ProcessImplementation
|| toImpl instanceof ProcessorImplementation
|| toImpl instanceof VirtualProcessorImplementation
|| toImpl instanceof DeviceImplementation
|| toImpl instanceof SystemImplementation) {
finalizeConnectionInstance = false;
}
} else if (toEnd instanceof SubprogramGroupAccess
&& ((SubprogramGroupAccess) toEnd).getKind() == AccessType.PROVIDES) {
if (toImpl instanceof DataImplementation
|| toImpl instanceof SubprogramGroupImplementation
|| toImpl instanceof ThreadImplementation
|| toImpl instanceof ThreadGroupImplementation
|| toImpl instanceof ProcessImplementation
|| toImpl instanceof ProcessorImplementation
|| toImpl instanceof VirtualProcessorImplementation
|| toImpl instanceof DeviceImplementation
|| toImpl instanceof SystemImplementation) {
finalizeConnectionInstance = false;
}
}
if (finalizeConnectionInstance) {
final ConnectionInfo clone = connInfo.cloneInfo();
clone.complete = true;
finalizeConnectionInstance(ci, clone, toFi);
}
This can probably be simplified, but it works this way and the intent seems much clearer like this.
Added unit tests.
Ran all the unit tests. Things are not okay. First test that fails is for Issue 667. The logic for the changes to appendSegment
is still incorrect.
I have to revisit the intention of the guard statements identified above in instantiateConnections
and appendSegments
. Looking at the code as it was before I attempted to fix it, it occurred to me there part of the test, (cat != THREAD && cat != PROCESSOR && cat != DEVICE && cat != VIRTUAL_PROCESSOR)
, seems to be an optimization to avoid going inside of components that aren't going to have useful internal connections. Whether this check was ever completely true, I'm not sure, but for the current AADL standard it makes no sense. Threads, processors, and devices can all contain subcomponents that have regular ports (never mind the provides access problem), and these ports can be connected to outgoing ports of the thread, processors, or device. The conditional clause above can disrupt this connection.
I can use a similar example as the ones above to demonstrate this. Here we have a thread with an abstract subcomponent, and the connection instances are messed up:
package JustAbstract
public
abstract A
features
p: in out event port;
end A;
thread T1
features
f1: in out event port;
end T1;
thread implementation T1.impl
subcomponents
sub: abstract A;
connections
c1: port sub.p <-> f1;
end T1.impl;
thread T2
features
f2: in out event port;
end T2;
thread implementation T2.impl
end T2.impl;
process p1
end p1;
process implementation p1.impl
subcomponents
MyT1: thread T1.impl;
MyT2: thread T2.impl;
connections
c2: port MyT1.f1 <-> MyT2.f2;
end p1.impl;
system Root
end Root;
system implementation Root.impl
subcomponents
MyP: process p1.impl;
end Root.impl;
end JustAbstract;
The connection instances generated are
But, if I take the model above and change the threads and processes to system
components,
package JustAbstract_System
public
abstract A
features
p: in out event port;
end A;
system T1
features
f1: in out event port;
end T1;
system implementation T1.impl
subcomponents
sub: abstract A;
connections
c1: port sub.p <-> f1;
end T1.impl;
system T2
features
f2: in out event port;
end T2;
system implementation T2.impl
end T2.impl;
system p1
end p1;
system implementation p1.impl
subcomponents
MyT1: system T1.impl;
MyT2: system T2.impl;
connections
c2: port MyT1.f1 <-> MyT2.f2;
end p1.impl;
system Root
end Root;
system implementation Root.impl
subcomponents
MyP: system p1.impl;
end Root.impl;
end JustAbstract_System;
the generated instance model is correct, which two connection instances corresponding to the bidirectionally of the original connections.
I discussed this with Lutz. The guard against threads, processors, and devices is meant to prevent connections ending at subprogram parameters. This is because of timing considerations. Lutz needs to get more clarification from Peter about this.
For now
Do not go into threads, processors, virtual processors or devices if the internal connection is a
parameter connection
.
Lots more things to create tests for:
subprogram
subcomponents: data, subprogram group, thread, thread group, process, processor (but it cannot have subprogram subcomponents??), virtual processor (but it cannot have subprogram subcomponents??), device (but it cannot have subprogram subcomponents??). Check that parameter connections are not allowed in these cases.Not sure what I need to do on the other end of the connection yet in appendSegment
. It's going to have to be a variation of the same test I put in instantiateConnections
, because it's just the same issue on the other end of the connection path.
May have gotten carried away in yesterday's discussion. The only things that can have subprogram calls and parameter connections are threads and other subprograms. So I don't see why we need to worry about memory, device, processor, or virtual processor.
Parser/verifier doesn't even allow parameter connections outside of thread or subprogram implementation classifiers.
Also, remember: parameter connections aren't dealt with at all in the instance model.
I tried attacking this problem in a more principle fashion as described above. The test we really care about is the connections inside the component are ParameterConnections
.
Again, I have changed things in 3 main locations, plus fiddled with some of the helper methods this time.
instantiateConnections()
I replaced
if (hasOutgoingFeatureSubcomponents
&& ((cat != THREAD && cat != PROCESSOR && cat != DEVICE && cat != VIRTUAL_PROCESSOR)
// in case of a provides bus access we want to
// start from the bus.
|| ((cat == PROCESSOR || cat == DEVICE || cat == MEMORY)
&& feature instanceof BusAccess
&& ((BusAccess) feature).getKind() == AccessType.PROVIDES)) {
connectedInside = isConnectionEnd(insideSubConns, feature);
destinationFromInside = isDestination(insideSubConns, feature);
}
with just
if (hasOutgoingFeatureSubcomponents
) {
connectedInside = isConnectionEnd(insideSubConns, feature);
destinationFromInside = isDestination(insideSubConns, feature);
}
but, I also changed isConnectionEnd
and isDestination
to ignore parameter connections:
public boolean isConnectionEnd(List<Connection> conns, Feature feature) {
List<Feature> features = feature.getAllFeatureRefinements();
for (Connection conn : conns) {
// ignore parameter connections
if (!(conn instanceof ParameterConnection)) {
if (features.contains(conn.getAllDestination()) || features.contains(conn.getAllSource())) {
return true;
}
if ((features.contains(conn.getAllDestinationContext())
|| features.contains(conn.getAllSourceContext()))) {
return true;
}
}
}
return false;
}
public boolean isDestination(List<Connection> conns, Feature feature) {
List<Feature> features = feature.getAllFeatureRefinements();
for (Connection conn : conns) {
// ignore parameter connections
if (!(conn instanceof ParameterConnection)) {
if (features.contains(conn.getAllDestination())
|| conn.isAllBidirectional() && features.contains(conn.getAllSource())) {
return true;
}
if ((features.contains(conn.getAllDestinationContext())
|| conn.isAllBidirectional() && features.contains(conn.getAllSourceContext()))) {
return true;
}
}
}
return false;
}
In appendSegment()
I changed
if (toEnd instanceof Parameter
|| finalComponent && !(toEnd instanceof FeatureGroup)) {
to just
if (toEnd instanceof Parameter) {
finalComponent
flag altogether, but it is used in the following else if
having to do with feature group features: } else if (finalComponent && toEnd instanceof FeatureGroup) {
I'm not sure what to do with this yet. I have a feeling this is also wrong and that it will prevent access features inside of feature groups from working correctly. I need a test case for this.
if (((toImpl instanceof ProcessorImplementation || toImpl instanceof DeviceImplementation
|| toImpl instanceof MemoryImplementation)
&& !(toEnd instanceof BusAccess
&& ((BusAccess) toEnd).getKind() == AccessType.PROVIDES))) {
final ConnectionInfo clone = connInfo.cloneInfo();
clone.complete = true;
finalizeConnectionInstance(ci, clone, toFi);
}
with
/*
* Issue 2032: If we get here then destination component has internal connections,
* not all of which are parameter connections. If there is at least one
* parameter connection, as indicated by the hasParameterConnection flag,
* we finalize the current connection, but also keep going with processing
* the internal connections along the current path.
*/
if (hasParameterConnection.get()) {
final ConnectionInfo clone = connInfo.cloneInfo();
clone.complete = true;
finalizeConnectionInstance(ci, clone, toFi);
}
where, earlier in the method I changed how the list of ingoing connections is computed. I replaced
List<Connection> conns = AadlUtil.getIngoingConnections(toImpl, toFeature);
with code that gets the connections, but filters out any parameter connections, but also makes note if there were any parameter connections.
final AtomicBoolean hasParameterConnection = new AtomicBoolean(false);
List<Connection> conns = AadlUtil.getIngoingConnections(toImpl, toFeature,
c -> {
if (c instanceof ParameterConnection) {
hasParameterConnection.set(true);
return false;
} else {
return true;
}
});
where I also changed AadlUtil.getIngoingConnections()
to take a predicate, (but also left a compatible original version in place):
public static EList<Connection> getIngoingConnections(ComponentImplementation cimpl, Feature feature) {
return getIngoingConnections(cimpl, feature, c -> true);
}
/**
* Get ingoing connections to subcomponents from a specified feature of the
* component impl
*
* @param feature component impl feature that is the source of a connection
* @param context the outer feature (feature group) or null
* @param useConnection Predicate that indicates if the connection should be added to the result. Each ingoing
* connection is tested before being added to the result. This predicate should return <code>true</code> if the
* connection should be added to the result.
* @return EList connections with feature as source
*/
public static EList<Connection> getIngoingConnections(ComponentImplementation cimpl, Feature feature,
Predicate<Connection> useConnection) {
EList<Connection> result = new BasicEList<Connection>();
// The local feature could be a refinement of the feature through which the connection enters the component
Feature local = (Feature) cimpl.findNamedElement(feature.getName());
List<Feature> features = local.getAllFeatureRefinements();
EList<Connection> cimplconns = cimpl.getAllConnections();
for (Connection conn : cimplconns) {
if (features.contains(conn.getAllSource()) && !(conn.getAllSourceContext() instanceof Subcomponent)
|| (conn.isAllBidirectional() && features.contains(conn.getAllDestination())
&& !(conn.getAllDestinationContext() instanceof Subcomponent))) {
if (useConnection.test(conn)) {
result.add(conn);
}
}
if ((features.contains(conn.getAllSourceContext())
|| (conn.isAllBidirectional() && features.contains(conn.getAllDestinationContext())))) {
if (useConnection.test(conn)) {
result.add(conn);
}
}
}
return result;
}
My earlier unit tests for this issue pass with these changes. I need to run all the tests now and see what I may have broken. I also want to add additional test of my own:
Good news, all the existing unit tests pass under this scenario!
I'm going to add some more tests specific to this issue (see above comment), and hopefully all will be fine.
Need to remember to get rid of any commented out code.
Added tests for
The following don't work right now:
They all fail the same way, with three connection instances:
Fixed the above problem. Had to update AadlUtil.hasOutgoingFeatureSubcomponents()
to consider virtual bus access.
public static boolean hasOutgoingFeatureSubcomponents(EList<? extends ComponentInstance> subcompinstances) {
for (ComponentInstance o : subcompinstances) {
EList<FeatureInstance> filist = o.getFeatureInstances();
for (FeatureInstance fi : filist) {
Feature f = fi.getFeature();
if (isOutgoingFeature(f)) {
return true;
}
}
// subcomponent can be access source
ComponentCategory cat = o.getCategory();
if (cat == DATA || cat == BUS || cat == VIRTUAL_BUS || cat == SUBPROGRAM || cat == SUBPROGRAM_GROUP) {
return true;
}
}
return false;
}
Added a ton of more tests, trying to capture all the cases where an access connection can be applied. Also added tests where we use access connections inside of feature groups. As predicted, not all of the feature group cases work. I believe this to be due to the use of the finalComponent
flag in appendSegment
as identified above.
Fixed feature group problems by removing the special section in appendSegment
that handles feature groups on final components:
} else if (finalComponent && toEnd instanceof FeatureGroup) {
// connection ends at a feature that is contained in a feature
// group
// of a thread, device, or (virtual) processor
FeatureInstance dstFi = toCi.findFeatureInstance(toFeature);
if (dstFi == null) {
error(toCi,
"Destination feature " + toFeature.getName() + " not found. No connection created.");
} else {
connInfo.complete = true;
finalizeConnectionInstance(ci, connInfo, dstFi);
}
Was able to remove the whole finalComonent
variable
All OSATE Unit tests pass with this change.
Added test case for the abstract
component issue that was identified above. Previous corrections to the code have fixed this issue.
Created test for the case where a thread has port feature that is internal connected via both parameter connection and a port connection. This is admittedly a convoluted case because it requires the use of an abstract
subcomponent in a way that it is probably not intended. In any case, we want to see two connection instances in this case:
This works as expected.
package StopAndGo
public
data D
end D;
subprogram Sub
features
p: in parameter D;
end sub;
abstract A
features
input: in data port D;
end A;
thread T1
features
output: out data port D;
end T1;
thread implementation T1.i
end T1.i;
thread T2
features
input: in data port D;
end T2;
thread implementation T2.i
subcomponents
s: subprogram Sub;
abs: abstract A;
calls
normal: {
call1: subprogram s;
};
connections
aa: port input -> abs.input;
bb: parameter input -> call1.p;
end T2.i;
process p
end p;
process implementation p.i
subcomponents
t1: thread t1.i;
t2: thread t2.i;
connections
cc: port t1.output -> t2.input;
end p.i;
system Root
end Root;
system implementation Root.impl
subcomponents
p: process p.i;
end Root.impl;
end StopAndGo;
Lutz pointed out that the above "stop and go" behavior is not desirable. I tested this on the master
branch, and there only a connection instance corresponding to cc
is created. This is the desired behavior. I have to go back to the working branch and disable whatever I did that allows this to happen with the current changes.
So the whole "stop and go" thing is enabled by getting rid of the finalComponent
flag in appendSegment
. But making it go away is not as simple as undoing that change because then the access connections in feature groups won't work. We need to add something somewhere that checks what is going on in the feature group?
I made a new version of the test that includes a subprogram access connection:
package StopAndGo
public
data D
end D;
subprogram ShareMe
end SHareMe;
subprogram Sub
features
p: in parameter D;
end sub;
abstract A
features
input: in data port D;
end A;
thread T1
features
output: out data port D;
shared_sub: provides subprogram access ShareMe;
end T1;
thread implementation T1.i
subcomponents
s: subprogram ShareMe;
connections
xxx: subprogram access s <-> shared_sub;
end T1.i;
thread T2
features
input: in data port D;
provided_sub: requires subprogram access ShareMe;
end T2;
thread implementation T2.i
subcomponents
s: subprogram Sub;
abs: abstract A;
calls
normal: {
call1: subprogram s;
};
connections
aa: port input -> abs.input;
bb: parameter input -> call1.p;
end T2.i;
process p
end p;
process implementation p.i
subcomponents
t1: thread t1.i;
t2: thread t2.i;
connections
cc: port t1.output -> t2.input;
dd: subprogram access t1.shared_sub <-> t2.provided_sub;
end p.i;
system Root
end Root;
system implementation Root.impl
subcomponents
p: process p.i;
end Root.impl;
end StopAndGo;
Currently this has 4 connection instances:
abstract
subcomponent that does into the thread to reach the abstract
subcomponent. [this is the connection that we do not want to have]Now to make sure that we don't break feature groups I made a version that uses a feature group with two features instead of the two features we have above:
package StopAndGo_fg
public
data D
end D;
subprogram ShareMe
end SHareMe;
subprogram Sub
features
p: in parameter D;
end sub;
abstract A
features
input: in data port D;
end A;
feature group FG1
features
output: out data port D;
shared_sub: provides subprogram access ShareMe;
end FG1;
feature group FG2
features
input: in data port D;
provided_sub: requires subprogram access ShareMe;
inverse of FG1
end FG2;
thread T1
features
fg1: feature group fg1;
end T1;
thread implementation T1.i
subcomponents
s: subprogram ShareMe;
connections
xxx: subprogram access s <-> fg1.shared_sub;
end T1.i;
thread T2
features
fg2: feature group fg2;
end T2;
thread implementation T2.i
subcomponents
s: subprogram Sub;
abs: abstract A;
calls
normal: {
call1: subprogram s;
};
connections
aa: port fg2.input -> abs.input;
bb: parameter fg2.input -> call1.p;
end T2.i;
process p
end p;
process implementation p.i
subcomponents
t1: thread t1.i;
t2: thread t2.i;
connections
cc: feature group t1.fg1 <-> t2.fg2;
end p.i;
system Root
end Root;
system implementation Root.impl
subcomponents
p: process p.i;
end Root.impl;
end StopAndGo;
I expected this to also have 4 connection instances. It does not. It has only 1, the subprogram access connection from t1
to t2
.
aa
to the abstract subcomponent, then I get back the subprogram access connection in the other direction.xxx
to the shared subcomponent in t1.i
then I have 3 connection instances: one to connection each feature of the feature group across t1
and t2
, and then one to go into t2
to connect to the abstract subcomponents (the one we don't want).I think there is something wrong in instantiateConnection
. I don't want to fix the finalComponent
issue in appendSegment
until I figure out what is going on with this situation.
There are several things that cause the above behavior with StopAndGo_fg
.
instantiateConnections()
causes problems because it won't start an opposite connection from a destination feature on a thread if that thread has internal connections to abstract items (the only kinds of non-access, non-parameter connections that would be allowed inside the thread in this case). This is why removing the aa
connection brings back the inverse access connection above. Follow up: Actually both these problems come from fact that I got rid of the "final component" checking in instantiateConnections()
. Urgh.
Okay, gotta rollback some of my earlier changes.
Changed the check in instantiateConnections()
to
if (hasOutgoingFeatureSubcomponents && (!isConnectionEndingCategory(cat)
|| (feature instanceof Access && ((Access) feature).getKind() == AccessType.PROVIDES))) {
connectedInside = isConnectionEnd(insideSubConns, feature);
destinationFromInside = isDestination(insideSubConns, feature);
}
where isConnectionEndingCategory()
is
private boolean isConnectionEndingCategory(final ComponentCategory cat) {
return cat == THREAD || cat == DEVICE || cat == PROCESSOR || cat == VIRTUAL_PROCESSOR;
}
This fixes some things. But now I have a connection instance between the two subprogram access features in the feature group: t1.fg1.shared_sub -> t2.fg2.provided_sub
. Not entirely sure what is causing this, but we don't want it. Should note though that this behavior now matches the behavior along the master
branch.
Can fix some more problems by putting back the final component checks in appendSegment()
(~line 509):
if (toEnd instanceof Parameter || finalComponent && !(toEnd instanceof Access)) {
// connection ends at a parameter or at a simple feature of a
// thread, device, or (virtual) processor
FeatureInstance dstFi = toCi.findFeatureInstance(toFeature);
if (dstFi == null) {
error(toCi,
"Destination feature " + toFeature.getName() + " not found. No connection created.");
} else {
connInfo.complete = true;
finalizeConnectionInstance(ci, connInfo, dstFi);
}
} else if (dstEmpty) {
(Don't need a separate check for feature groups like there used to be be because the executed code was identical.)
However, we do need to add some specialness for feature groups because we need to treat feature groups that have access features the same as we treat access features.
Further tweaked appendSegment()
. Changed the part around line 509 to
Feature toFeature = (Feature) toEnd;
final boolean endHasAccessFeatures = endHasAccessFeatures(toCi, toEnd);
if (toEnd instanceof Parameter || finalComponent && !endHasAccessFeatures) {
// connection ends at a parameter or at a simple feature of a
// thread, device, or (virtual) processor
FeatureInstance dstFi = toCi.findFeatureInstance(toFeature);
if (dstFi == null) {
error(toCi,
"Destination feature " + toFeature.getName() + " not found. No connection created.");
} else {
connInfo.complete = true;
finalizeConnectionInstance(ci, connInfo, dstFi);
}
} else if (dstEmpty) {
where endHasAccessFeatures()
is
private boolean endHasAccessFeatures(final ComponentInstance ci, final ConnectionEnd end) {
if (end instanceof Access) {
return true;
} else if (end instanceof FeatureGroup) {
final FeatureGroup fg = (FeatureGroup) end;
final FeatureInstance fgInstance = ci.findFeatureInstance(fg);
for (final FeatureInstance fi : fgInstance.getFeatureInstances()) {
if (fi.getFeature() instanceof Access) {
return true;
}
}
return false;
} else {
return false;
}
}
Then I changed the part that gets the internal "ingoing connections" to filter the connections differently. Namely, if the connection end is part of a final component and but has access features, we only want to return access connections. This is necessary to keep from connecting to the port features inside of feature groups on final components.
/*
* Issue 2032: Get the connections internal to the destination component that connect
* to the feature. Two cases here. (1) If the component is final (thread/device/processor) but
* the end we are starting from has access features, then we are only interested in access connections
* internal to the component. (2) otherwise we want all the internal connections except for the parameter
* connections. Either way, we need to know if there are any parameter connections.
*/
final AtomicBoolean hasParameterConnection = new AtomicBoolean(false);
List<Connection> conns = AadlUtil.getIngoingConnections(toImpl, toFeature,
(finalComponent && endHasAccessFeatures) ? c -> {
if (c instanceof ParameterConnection) {
hasParameterConnection.set(true);
return false;
} else if (c instanceof AccessConnection) {
return true;
} else {
return false;
}
} :
c -> {
if (c instanceof ParameterConnection) {
hasParameterConnection.set(true);
return false;
} else {
return true;
}
});
Now I still have one extra connection instance in the feature group case, the connection instance between the provides and requires access features. This shouldn't be there, and doesn't appear in the case where I don't use feature groups. I'm not sure where it is coming from. Going to set a breakpoint on finalizeConnectionInstance()
to figure this one out.
Okay the unwanted connection between the access features of the feature groups is generated by finalizeConnectionInstance()
as it matches the two feature groups together. This is not trivial to deal with. We want to have it not generate the connection for the features but we may not know yet that we want this. We only know the connection becomes redundant/undesirable after a connection instance is made all the two the shared subprogram.
This behavior was not introduced by trying to fix this bug. It exists on the master branch as well. Probably should introduce a separate issue for it.
Started going through my unit tests for this issue and the feature groups were being weird again. Problem is that I forgot I need to go into the feature group at the start in instantiateConnections
just like to do at the end in appendSegment
to check if the feature group has access connections.
Changed the block in instantiateConnections
to
if (hasOutgoingFeatureSubcomponents
&& (!isConnectionEndingCategory(cat) || endHasAccessFeatures(ci, feature))) {
connectedInside = isConnectionEnd(insideSubConns, feature);
destinationFromInside = isDestination(insideSubConns, feature);
}
and I also updated endHasAccessFeatures()
to check for provides access only (as was in the original code on the master
branch):
private boolean endHasAccessFeatures(final ComponentInstance ci, final ConnectionEnd end) {
if (end instanceof Access) {
return ((Access) end).getKind() == AccessType.PROVIDES;
} else if (end instanceof FeatureGroup) {
final FeatureGroup fg = (FeatureGroup) end;
final FeatureInstance fgInstance = ci.findFeatureInstance(fg);
for (final FeatureInstance fi : fgInstance.getFeatureInstances()) {
if (fi.getFeature() instanceof Access) {
return ((Access) fi.getFeature()).getKind() == AccessType.PROVIDES;
}
}
return false;
} else {
return false;
}
}
This fixed the feature group problems in my unit tests.
After the above changes the StopAndGo_fg
example only has two connection instances, those for the shared subpgrogram. These seems wrong....
The new problems with StopAndGo_fg
are caused by these lines in instantiateConnections
:
if (hasOutgoingFeatureSubcomponents
&& (!isConnectionEndingCategory(cat) || endHasAccessFeatures(ci, feature))) {
connectedInside = isConnectionEnd(insideSubConns, feature);
destinationFromInside = isDestination(insideSubConns, feature);
}
and the interaction with the conditional in the following for
-loop:
if (!(destinationFromInside || conn.isAllBidirectional() && connectedInside)) {
specifically, the test endHasAccessFeatures
. This is fine for singular connections, but for feature group it is no good because a feature group can have additional ports that are not access connections, and thus we still want the above conditional to be true
even though it would be false
in the purely access feature case.
We need to separately keep track of what kind of features are present on the end point and explicitly test for the one that we care about at each point.
There is probably an analogous place in appendSegment()
that checks the feature at the other end of the connection.
According to Lutz, we should ignore port connections inside of thread:
So I talked to Peter and he agrees that it is acceptable to ignore connections to/from abstract components inside threads, etc. In the future we may add validations that make sure that such an abstract component can have only features that are valid for concrete components in threads, so, for example, ports on abstract components inside threads would no longer be allowed.
Since we are considering thread
, device
, processor
and virtual processor
as connection ending components, I think we should treat them the same here and only go inside them for access connections.
Things get uglier and uglier. To prevent the creation of "upward" connections from the output ports of abstract subcomponents to the ports of their containing "connection ending component", we need to add checks to addSegment()
. This cannot be handled in createConnectionIntances()
. The analogous "downward" check will be later on inside of addSegment()
.
Added
} else {
toFi = ci.findSubcomponentInstance((Subcomponent) toEnd);
}
/*
* Issue 2032: We do not want connections that go from abstract subcomponent to the ports of
* their containing components if the containing component is final. We specifically are
* checking that the connection starts at a port feature and ends at a feature that is a feature
* of the containing component and the containing component is a connection ending component. We don't
* have to check that the end feature is a port because AADL semantics guarantee that it will be.
*/
if (fromFi instanceof FeatureInstance && ((FeatureInstance) fromFi).getFeature() instanceof Port
&& toFi.eContainer().equals(ci) && isConnectionEndingCategory(ci.getCategory())) {
return;
}
try {
boolean[] keep = { false };
boolean valid = connInfo.addSegment(newSegment, fromFi, toFi, ci, goOpposite, keep);
if (!keep[0]) {
return;
}
into appendSegment()
to drop upping abstract component connections. Seems to work.
Can now focus on making changes to the end of appendSegment
to mirror the recent changes to instantiateConnections()
and the above change to appendSegment
.
fixed appendSegment()
as above. Unit tests for this issue pass. I updated the stop and go tests to deal with the abstract connections:
Unit tests (all) pass. Going to create pull request.
This example is derived from the example in Issue #1941. When the system below is instantiated, the process instance
MyP
has three connection instances, two of which should not be generated. Furthermore, it should have an additional connection instance that is missing.The instance model has the connection instances (with the given connection references):
Only the second connection instance
MyT1.CompAvg -> MyT2.ext_sub
should exist. The other two connections are incomplete nonsense.Furthermore, the connection between
CompAvg
andext_sub
should be bidirectional, so there should be a connection instancesMyT2.ext_sub -> MyT1.CompAvg
, but there currently is not one.