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

SSのテレメ異常EHのカウンタをSSごとに設定 #289

Closed
wants to merge 2 commits into from

Conversation

seki-hiro
Copy link
Member

@seki-hiro seki-hiro commented May 7, 2024

Issue

N/A

詳細

SS4つはI2Cバスを共有しているので、あるSSでテレメエラーが起きた時に、他のSSでもテレメエラーとなりうる。4つのEHが同時に発火して予期しない挙動となることを防ぐために、EH発火の条件をSS毎にずらす。

検証結果

ビルドチェック (どちらもチェック)

  • SILSでのビルドチェックに通った(CIで確認)
  • vMicroでのビルドチェックに通った

動作確認チェック (いずれかをチェック)

  • SILSでアルゴリズムが想定通りに動いた
  • 実機でアルゴリズムが想定通りに動いた
  • (テレコマ試験の場合)コマンドファイルを使った試験をパスした

試験結果詳細記述場所 or 詳細ログ保存場所へのリンク

SILSでevent_handler_rule_tlmcmd.cにて、各SSで別々のカウンタ閾値が設定されていることが確認できた。

補足

N/A

@seki-hiro seki-hiro added ✈️ priority::medium priority medium 🐟 patch Patch Update labels May 7, 2024
@seki-hiro seki-hiro self-assigned this May 7, 2024
@seki-hiro seki-hiro requested review from sksat and a team as code owners May 7, 2024 01:00
@seki-hiro seki-hiro requested review from 200km, suzuki-toshihir0, t-hosonuma and conjikidow and removed request for a team May 7, 2024 01:00
@seki-hiro seki-hiro changed the title WIP: SSのテレメ異常EHのカウンタをSSごとに設定 SSのテレメ異常EHのカウンタをSSごとに設定 May 7, 2024
@seki-hiro seki-hiro added this to the v10.0.0 Major update milestone May 7, 2024
const uint16_t FDIR_PARAMETERS_tlm_error_eh_reset_count_threshold_nanossoc_d60_0 = 250;
const uint16_t FDIR_PARAMETERS_tlm_error_eh_reset_count_threshold_nanossoc_d60_1 = 260;
const uint16_t FDIR_PARAMETERS_tlm_error_eh_reset_count_threshold_nanossoc_d60_2 = 270;
const uint16_t FDIR_PARAMETERS_tlm_error_eh_reset_count_threshold_nanossoc_d60_3 = 280;
Copy link
Member

Choose a reason for hiding this comment

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

[MUST] EHをサンセンサ1つずつに対して定義する場合、サンセンサの個数が最大6個までで可変なことを考えて実装する必要があります。

Copy link
Member

Choose a reason for hiding this comment

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

この修正起因というか、昔サンセンサ個数を任意に変更にした時に本来はここに最大6個分までのEHを登録する必要があったという感じですね。すみません。

Copy link
Member Author

Choose a reason for hiding this comment

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

はい、ただEH自体は1つではなく4つ定義されていて、6つにするのはTODOとなっています。
https://github.com/ut-issl/c2a-aobc/pull/289/files#diff-4765fd1a98eafaac9e712a19389fa46d3bd43f34f5164eca121f0acbe923adc6R134

なので、影響範囲が大きそうな4->6の拡張をするのではなく、ここでは現状の4つのEHを独立で定義できるように微修正するにとどめたいのですが、よろしいでしょうか?

Copy link
Member Author

Choose a reason for hiding this comment

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

https://github.com/ut-issl/c2a-aobc-onglaisat/pull/53#issuecomment-2099951115

こっちのコメントを後に見ました。このスレッドは一旦保留でお願いします。

Copy link
Member

Choose a reason for hiding this comment

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

今回は4つのままで進めるということでOKです。issue登録だけしておいてもらえると助かります。

Copy link
Member Author

Choose a reason for hiding this comment

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

@200km 定義するのは4つのまま進めるけど、デフォルトで有効化するのは1つということで、各SSでカウントをずらすモチベーションがなくなってしまったので、このPRはクローズさせていただきます。

Copy link
Member Author

Choose a reason for hiding this comment

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

ここでissue登録しました。#294

@200km 200km self-requested a review May 9, 2024 13:24
@seki-hiro seki-hiro closed this May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✈️ priority::medium priority medium 🐟 patch Patch Update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants