virtualsatellite / VirtualSatellite4-CEF

Virtual Satellite 4 - CEF Application
Eclipse Public License 2.0
9 stars 3 forks source link

Feature/209 add of initial class for cefx template and study template for default subsystems #221

Open DimitriDiantos opened 6 months ago

DimitriDiantos commented 6 months ago

To accomplish this task, we followed the steps outlined below:

Created the CEF X package. Developed various classes in accordance with the principles of the CEF Template and implemented them. Incorporated the different components of the template into the template Menu to ensure visibility. Updated the "Create DLR CEFX Equipment" command. Addressed and resolved any errors encountered during the process.

Default subsystems encompasses: Power Structure AOCS Payload DataHandling.

franzTobiasDLR commented 6 months ago

Hi Dimitri,

didn’t we close this PR? I thought we decided that all features of this PR are also in this: https://github.com/virtualsatellite/VirtualSatellite4-CEF/pull/209

Why do we have it here again?

Cheers Tobi

Von: Dimitri Ngatcha Pokouane @.> Gesendet: Donnerstag, 14. März 2024 10:55 An: virtualsatellite/VirtualSatellite4-CEF @.> Cc: Franz, Tobias @.>; Review requested @.> Betreff: Re: [virtualsatellite/VirtualSatellite4-CEF] Feature/209 add of initial class for cefx template and study template for default subsystems (PR #221)

@DimitriDiantoshttps://github.com/DimitriDiantos requested your review on: #221https://github.com/virtualsatellite/VirtualSatellite4-CEF/pull/221 Feature/209 add of initial class for cefx template and study template for default subsystems.

— Reply to this email directly, view it on GitHubhttps://github.com/virtualsatellite/VirtualSatellite4-CEF/pull/221#event-12115564570, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AL2YSL4K3MC55H3IFX5OK43YYFXYBAVCNFSM6AAAAABEVYJ5JOVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSGEYTKNJWGQ2TOMA. You are receiving this because your review was requested.Message ID: @.**@.>>

codecov[bot] commented 6 months ago

Codecov Report

Merging #221 (ddddf31) into development (993494a) will not change coverage. The diff coverage is n/a.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/virtualsatellite/VirtualSatellite4-CEF/pull/221/graphs/tree.svg?width=650&height=150&src=pr&token=wihKOndYOp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=virtualsatellite)](https://app.codecov.io/gh/virtualsatellite/VirtualSatellite4-CEF/pull/221?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=virtualsatellite) ```diff @@ Coverage Diff @@ ## development #221 +/- ## ============================================== Coverage 88.00% 88.00% Complexity 518 518 ============================================== Files 74 74 Lines 1750 1750 Branches 217 217 ============================================== Hits 1540 1540 Misses 134 134 Partials 76 76 ```
DimitriDiantos commented 6 months ago

Hi Dimitri, didn’t we close this PR? I thought we decided that all features of this PR are also in this: #209 Why do we have it here again? Cheers Tobi Von: Dimitri Ngatcha Pokouane @.> Gesendet: Donnerstag, 14. März 2024 10:55 An: virtualsatellite/VirtualSatellite4-CEF @.> Cc: Franz, Tobias @.>; Review requested @.> Betreff: Re: [virtualsatellite/VirtualSatellite4-CEF] Feature/209 add of initial class for cefx template and study template for default subsystems (PR #221) @DimitriDiantoshttps://github.com/DimitriDiantos requested your review on: #221<#221> Feature/209 add of initial class for cefx template and study template for default subsystems. — Reply to this email directly, view it on GitHub<#221 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AL2YSL4K3MC55H3IFX5OK43YYFXYBAVCNFSM6AAAAABEVYJ5JOVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSGEYTKNJWGQ2TOMA. You are receiving this because your review was requested.Message ID: @.**@.>>

Hi Dimitri, didn’t we close this PR? I thought we decided that all features of this PR are also in this: #209 Why do we have it here again? Cheers Tobi Von: Dimitri Ngatcha Pokouane @.> Gesendet: Donnerstag, 14. März 2024 10:55 An: virtualsatellite/VirtualSatellite4-CEF @.> Cc: Franz, Tobias @.>; Review requested @.> Betreff: Re: [virtualsatellite/VirtualSatellite4-CEF] Feature/209 add of initial class for cefx template and study template for default subsystems (PR #221) @DimitriDiantoshttps://github.com/DimitriDiantos requested your review on: #221<#221> Feature/209 add of initial class for cefx template and study template for default subsystems. — Reply to this email directly, view it on GitHub<#221 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AL2YSL4K3MC55H3IFX5OK43YYFXYBAVCNFSM6AAAAABEVYJ5JOVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSGEYTKNJWGQ2TOMA. You are receiving this because your review was requested.Message ID: @.**@.>>

Hi Dimitri, didn’t we close this PR? I thought we decided that all features of this PR are also in this: #209 Why do we have it here again? Cheers Tobi Von: Dimitri Ngatcha Pokouane @.> Gesendet: Donnerstag, 14. März 2024 10:55 An: virtualsatellite/VirtualSatellite4-CEF @.> Cc: Franz, Tobias @.>; Review requested @.> Betreff: Re: [virtualsatellite/VirtualSatellite4-CEF] Feature/209 add of initial class for cefx template and study template for default subsystems (PR #221) @DimitriDiantoshttps://github.com/DimitriDiantos requested your review on: #221<#221> Feature/209 add of initial class for cefx template and study template for default subsystems. — Reply to this email directly, view it on GitHub<#221 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AL2YSL4K3MC55H3IFX5OK43YYFXYBAVCNFSM6AAAAABEVYJ5JOVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSGEYTKNJWGQ2TOMA. You are receiving this because your review was requested.Message ID: @.**@.>>

Because i couldn't push the update on github. I got a message like the failed. You can see the picture attached.

Screenshot 2024-03-12 144701
dellerDLR commented 6 months ago

Hi Dimitri, didn’t we close this PR? I thought we decided that all features of this PR are also in this: #209 Why do we have it here again? Cheers Tobi Von: Dimitri Ngatcha Pokouane @.**> Gesendet: Donnerstag, 14. März 2024 10:55 An: virtualsatellite/VirtualSatellite4-CEF @.**> Cc: Franz, Tobias @.**>; Review requested @.**> Betreff: Re: [virtualsatellite/VirtualSatellite4-CEF] Feature/209 add of initial class for cefx template and study template for default subsystems (PR #221) @DimitriDiantoshttps://github.com/DimitriDiantos requested your review on: #221<#221> Feature/209 add of initial class for cefx template and study template for default subsystems. — Reply to this email directly, view it on GitHub<#221 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AL2YSL4K3MC55H3IFX5OK43YYFXYBAVCNFSM6AAAAABEVYJ5JOVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSGEYTKNJWGQ2TOMA. You are receiving this because your review was requested.Message ID: @.**@.**>>

Hi Dimitri, didn’t we close this PR? I thought we decided that all features of this PR are also in this: #209 Why do we have it here again? Cheers Tobi Von: Dimitri Ngatcha Pokouane @.**> Gesendet: Donnerstag, 14. März 2024 10:55 An: virtualsatellite/VirtualSatellite4-CEF @.**> Cc: Franz, Tobias @.**>; Review requested @.**> Betreff: Re: [virtualsatellite/VirtualSatellite4-CEF] Feature/209 add of initial class for cefx template and study template for default subsystems (PR #221) @DimitriDiantoshttps://github.com/DimitriDiantos requested your review on: #221<#221> Feature/209 add of initial class for cefx template and study template for default subsystems. — Reply to this email directly, view it on GitHub<#221 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AL2YSL4K3MC55H3IFX5OK43YYFXYBAVCNFSM6AAAAABEVYJ5JOVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSGEYTKNJWGQ2TOMA. You are receiving this because your review was requested.Message ID: @.**@.**>>

Hi Dimitri, didn’t we close this PR? I thought we decided that all features of this PR are also in this: #209 Why do we have it here again? Cheers Tobi Von: Dimitri Ngatcha Pokouane @.**> Gesendet: Donnerstag, 14. März 2024 10:55 An: virtualsatellite/VirtualSatellite4-CEF @.**> Cc: Franz, Tobias @.**>; Review requested @.**> Betreff: Re: [virtualsatellite/VirtualSatellite4-CEF] Feature/209 add of initial class for cefx template and study template for default subsystems (PR #221) @DimitriDiantoshttps://github.com/DimitriDiantos requested your review on: #221<#221> Feature/209 add of initial class for cefx template and study template for default subsystems. — Reply to this email directly, view it on GitHub<#221 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AL2YSL4K3MC55H3IFX5OK43YYFXYBAVCNFSM6AAAAABEVYJ5JOVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSGEYTKNJWGQ2TOMA. You are receiving this because your review was requested.Message ID: @.**@.**>>

Because i couldn't push the update on github. I got a message like the failed. You can see the picture attached. Screenshot 2024-03-12 144701

Hey Dima, did you try to pull (resolve conflicts if needed) and push again? The same you have to do with this branch (if we proceed here) to make it up to date with the dev branch

dellerDLR commented 6 months ago

From a code point of view it looks good to me. But the template button is grayed out, eventhough the needed concepts are activated: image

dellerDLR commented 6 months ago

Also, dont forget to update this branch, so it has the current changes of development

dellerDLR commented 6 months ago

Okay, the problem was that the CommandHelper is using the new discipline API. @DimitriDiantos Do not forget to use the release target platform, since this feature will be included in the 4.16.2 release. As a result you have to set the discipline name as follows `newDiscipline.setUser(UserRegistry.getInstance().getUserName());.

Furthermore the ElementDefinitions do not have PowerEquipment parameters. A bonus would be also to have a Systemmodes Subsystem. But we should focus on the things above first

DimitriDiantos commented 6 months ago

From a code point of view it looks good to me. But the template button is grayed out, eventhough the needed concepts are activated: image

Hello Dennis,

Thanks for the feedback, but I just tried again and everything is working fine on my end. Please take a look at the attached photo. Maybe you'll need to try again or re-import or check out the branch again.

DimitriDiantos commented 6 months ago

Okay, the problem was that the CommandHelper is using the new discipline API. @DimitriDiantos Do not forget to use the release target platform, since this feature will be included in the 4.16.2 release. As a result you have to set the discipline name as follows `newDiscipline.setUser(UserRegistry.getInstance().getUserName());.

Furthermore the ElementDefinitions do not have PowerEquipment parameters. A bonus would be also to have a Systemmodes Subsystem. But we should focus on the things above first

Thank you for the feedback. Initially, I used the target release platform and made the modifications accordingly. However, when I pushed to GitHub, an error occurred indicating that 'setUser' did not exist. Consequently, I had to switch to the target development platform. If you could verify this, you will see.

DimitriDiantos commented 6 months ago

Okay, the problem was that the CommandHelper is using the new discipline API. @DimitriDiantos Do not forget to use the release target platform, since this feature will be included in the 4.16.2 release. As a result you have to set the discipline name as follows `newDiscipline.setUser(UserRegistry.getInstance().getUserName());.

Furthermore the ElementDefinitions do not have PowerEquipment parameters. A bonus would be also to have a Systemmodes Subsystem. But we should focus on the things above first

DimitriDiantos commented 6 months ago

From a code point of view it looks good to me. But the template button is grayed out, eventhough the needed concepts are activated: image

Hello Dennis,

Thanks for the feedback, but I just tried again and everything is working fine on my end. Please take a look at the attached photo. Maybe you'll need to try again or re-import or check out the branch again.

CEFX
DimitriDiantos commented 6 months ago

Okay, the problem was that the CommandHelper is using the new discipline API. @DimitriDiantos Do not forget to use the release target platform, since this feature will be included in the 4.16.2 release. As a result you have to set the discipline name as follows `newDiscipline.setUser(UserRegistry.getInstance().getUserName());.

Furthermore the ElementDefinitions do not have PowerEquipment parameters. A bonus would be also to have a Systemmodes Subsystem. But we should focus on the things above first

Thanks for your comment. Initially, I used the target release platform and made the modifications accordingly. However, when I pushed to GitHub, an error occurred indicating that 'setUser' did not exist. Consequently, I had to switch to the target development platform. See the picture attached below.

bug
DimitriDiantos commented 6 months ago

The looks good to me. Following things I noticed:

  1. The Subsystem under the CT:System has PowerParameters. I think we agreed on only adding mass parameters by default. same with SystemPowerParameters.
  2. When adding a Subsystem or Equipment Power as well as TemperatureParameters are getting added. Should this be the default or only the mass parameters?
  3. An Equipment can be added on Subsystemlevel. But I guess this should be possible due to the dynamic structure of the Product Structure Concept? What do you think @franzTobiasDLR

Despite that I think it can be merged :)

Okay Dennis :). i modified it.

franzTobiasDLR commented 1 month ago

@dellerDLR can you check if we can merge this PR?