Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MaaS model correct conversion #3

Merged
merged 18 commits into from
Jun 25, 2024
Merged

MaaS model correct conversion #3

merged 18 commits into from
Jun 25, 2024

Conversation

Nicolas-Boltz
Copy link
Member

Add MaaS model and tmp test code

@uuqjz
Copy link
Collaborator

uuqjz commented Jun 15, 2024

Ive removed the additional test function and replaced it with this line:
testSpecificModel("MaaS_Ticket_System_base", "MaaS", modelLocation, "maas.json", null);
As expected the existing tests fail for maas, indicating inconsistencies

@uuqjz
Copy link
Collaborator

uuqjz commented Jun 15, 2024

Ive also added the CWA so we can check when the converters is fixed that all test pass on TravelPlanner, MaaS and CWA

@uuqjz
Copy link
Collaborator

uuqjz commented Jun 16, 2024

Ive fixed the existing tests, so that they run on TravelPlanner, CWA and MaaS. This doesn't solve the issue with the missing behaviour but makes a better starting point

@uuqjz
Copy link
Collaborator

uuqjz commented Jun 16, 2024

Forwarding works now correctly
Bildschirmfoto 2024-06-16 um 14 45 36

@uuqjz
Copy link
Collaborator

uuqjz commented Jun 16, 2024

@Nicolas-Boltz For Normal assignments: Shall i just set the node charactersits at every outpin? Akin to the micro converter

@uuqjz
Copy link
Collaborator

uuqjz commented Jun 19, 2024

@01Parzival10 @Entenwilli I am done fixing small stuff, you can review this PR now.

@Entenwilli
Copy link
Member

Entenwilli commented Jun 24, 2024

Currently, this PR is hard to evaluate, as assigning a forward assignment to every node (including the PCM Source Node) will lead to no TFGs as the finder cannot determine a source and recurses endlessly

Copy link
Member

@Entenwilli Entenwilli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some smaller review comments, overall it looks good

@01Parzival10
Copy link
Contributor

01Parzival10 commented Jun 24, 2024

Currently, this PR is hard to evaluate, as assigning a forward assignment to every node (including the PCM Source Node) will lead to no TFGs as the finder cannot determine a source and recurses endlessly

Is this an actual issue tho? Are the DFD's created by the Converter actually supposed to be put into the analysis again?

Because #170 will not solve this. It will probably lead to single node unconnected TFG's since All the assignments will be independent of their input pins

@Entenwilli
Copy link
Member

Hmm, also true.... From what I have gathered so far, the created DFD seems fine, so there is no need to look at the TFGs of the generated model

@uuqjz
Copy link
Collaborator

uuqjz commented Jun 24, 2024

The DFDs created by the converter are put back into the (dfd)analysis that's the whole point.
For microsecend this works just fine, and i thought with the changes of #170 it should work for pcm too

@Entenwilli
Copy link
Member

The DFDs created by the converter are put back into the (dfd)analysis that's the whole point. For microsecend this works just fine, and i thought with the changes of #170 it should work for pcm too

If @01Parzival10 is correct (and I agree he is), DataFlowAnalysis/DataFlowAnalysis#170 will indeed not fix this. To get usable TFGs from the Finder, Forwarding Assignments are most likely needed

@uuqjz uuqjz requested a review from Entenwilli June 24, 2024 13:08
@uuqjz
Copy link
Collaborator

uuqjz commented Jun 24, 2024

Lets discuss that tomorrow and adress it in a separate PR.
So that this one just contains the fix and can be merged

@uuqjz
Copy link
Collaborator

uuqjz commented Jun 24, 2024

@01Parzival10 Shall i wait for your review?

@uuqjz uuqjz merged commit d9713c8 into main Jun 25, 2024
1 check passed
@uuqjz uuqjz deleted the behaviorIssue branch June 25, 2024 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MaaS model is not correctly converted
4 participants