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

Task #206 fix the bug in the equation where mass and power are being #218

Open
wants to merge 6 commits into
base: development
Choose a base branch
from

Conversation

DimitriDiantos
Copy link

@DimitriDiantos DimitriDiantos commented Mar 6, 2024

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.

Copy link

codecov bot commented Mar 6, 2024

Codecov Report

Merging #218 (7e534b1) into development (993494a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@              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           
Files Coverage Δ
...sat/model/extension/cefx/migrator/Migrator1v2.java 100.00% <100.00%> (ø)

Copy link
Member

@franzTobiasDLR franzTobiasDLR left a 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

Copy link
Member

@franzTobiasDLR franzTobiasDLR left a 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

@DimitriDiantos
Copy link
Author

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"/>
Copy link
Contributor

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

Copy link
Contributor

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

Copy link
Contributor

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 {
Copy link
Contributor

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

Copy link
Contributor

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

Copy link
Contributor

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the ToDos

Copy link
Contributor

Choose a reason for hiding this comment

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

should be reverted

Copy link
Contributor

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

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.

CEF: Modeling of subequipment causes wrong summary of mass and power
3 participants