-
Notifications
You must be signed in to change notification settings - Fork 3
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
Task #206 fix the bug in the equation where mass and power are being #218
base: development
Are you sure you want to change the base?
Task #206 fix the bug in the equation where mass and power are being #218
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## development #218 +/- ##
==============================================
Coverage 88.00% 88.00%
- Complexity 518 519 +1
==============================================
Files 74 75 +1
Lines 1750 1751 +1
Branches 217 217
==============================================
+ Hits 1540 1541 +1
Misses 134 134
Partials 76 76
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good allready! Thanks
....extension.cefx.test/src/de/dlr/sc/virsat/model/extension/cefx/migrator/Migrator1v2Test.java
Outdated
Show resolved
Hide resolved
...sat.model.extension.cefx/src/de/dlr/sc/virsat/model/extension/cefx/migrator/Migrator1v2.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Looks like we need some more updates here:
Places where we need to update the summary calculation:
- For System, Subsystem and Equipment
- For massTotal & massTotalWithMargin
- powerAvgWithMargin
Maybe check if there are other summry functions in the concept
Hi Tobi, thanks for comment. I fixed it. It's okay now. |
@@ -39,20 +39,20 @@ | |||
</left> | |||
<right xsi:type="dvlm_calc:ReferencedDefinitionInput" reference="de.dlr.sc.virsat.model.extension.cef.interfaces.EquipmentDataParameters.DataDutyCycle"/> | |||
</expression> | |||
<result xsi:type="dvlm_calc:EquationIntermediateResult" uuid="b4f41b65-ac2d-4bc3-8cb0-853c76c57529" name="DataCombinedDutyCycle"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cef concept should not change at all, since only the cefx is changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please replace this with the old version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is the old year and old institute name added?
2008-2024 German Aerospace Center (DLR), Institute for Software Technology, Germany is right
displayname "CEF-Extended" | ||
version 1.2 | ||
description "VirSat DLR CEF Concept for extended Product Structures" | ||
beta { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
beta should also be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And try to commit it with the right whitspaces
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please replace this with the old version like above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the ToDos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be reverted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@franzTobiasDLR the version 1.1 of the concept should not have any parametername changes, right?
They should be part of the version 1.2
We adjust the reference to only consider the mass and power of the subequipments
without including the main equipment's values again. and we will do similarly for power parameters.