Support Linux for SILS with WINGS #76

conjikidow · 2023-06-02T13:07:35Z

概要

Linux で WINGS を含めた SILS を回せるようにした。
またAOCSの用途に合わせて不要な部分を削除し，その他SILS関連のリファクタリングを行った。

Issue

NA

詳細

Linux 上だと

option(USE_SCI_COM "Use SCI_COM" ON)

とした際に，元のコードでは

#include <Windows.h>

の部分でビルドエラーとなっていた（Windows 依存なので当然）。
これをマクロで環境ごとに分け，Linux 上でも動くようにした。

また，MOBC用のCCSDS_SILS_SCI_IF.cの削除，uart_sils_sci_if.cppのUART_SILS.cへの統合，命名の修正，ポート設定の外部ファイル化等のリファクタリングを行った。

検証結果

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

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

動作確認チェック (全てチェック)

WindowsでWINGSを用いたSILSが想定通りに動いた
LinuxでWINGSを用いたSILSが想定通りに動いた

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

NA

影響範囲

以下には影響しない想定である

Windows環境での実行
WINGSを介さないSILS
実機

ただし，build optionのUSE_SCI_COMを廃止し，USE_SCI_COM_UARTをUSE_UART_COMに変更した。

補足

docker 化するのであれば別途修正が必要。

200km · 2023-06-02T13:13:13Z

CI回らないのなんでなんだろう。まだ設定が間違っているところがあるのかな、、、

200km · 2023-06-02T13:16:21Z

PRありがとうございます。詳細は見ておきますが、ちょっと気になるところをコメントしておきます。
UART関連は修正してくれているuart_sils_sci_if.cppだけでなく、UART_SILS.c もあり、SILS/HILSのコンポーネント通信は基本的にこれを使っていると思います。uart_sils_sci_if.cppを廃止してUART_SILS.cに統一していったほうが管理も楽なのかなと思うのですがどうでしょう？

conjikidow · 2023-06-02T13:16:53Z

ブランチ切ったのが昔だったのでそれが原因かなと思いrebaseしましたが，CIは改善されないですね......

200km · 2023-06-02T13:17:56Z

CI

うーん。coreアップデートのときはこれで動くようになって直したと思ったんですけどね。。。もう少し見てみます。

conjikidow · 2023-06-02T13:24:19Z

uart_sils_sci_if.cppを廃止してUART_SILS.cに統一していったほうが管理も楽なのかなと思うのですがどうでしょう？

実際に修正したのは去年の夏頃で正直中身もそこまで覚えていないので，現段階では具体的な回答はできないです，すみません。
詳細についてはdocker化なども含めて今後の方針が決まってから議論できたらと思います（しばらく手が回らないと思うのでそうさせてください）。

200km · 2023-06-02T13:24:45Z

CIはこれでできるようになるはず、、、

200km · 2023-06-02T13:27:09Z

実際に修正したのは去年の夏頃で

わかりました。では、大きな方針転換の議論は避けてreviewしますね。
時間が立ってこうなるのはもったいないので、早めにPR出してみんなで議論していくのが良いと思うので、これからはそうしてみてください。

conjikidow · 2023-06-02T13:34:25Z

すみません，ありがとうございます。

200km · 2023-06-02T13:46:42Z

この部分は、実は姿勢系やS2Eは関係なく、CDH、C2A、WINGSの部分になるので、CDHレビューは必須になる気がしますね。

(今回対応しなくても良いですが)背景を書いておくと、

UART_SILS.cはAOCSがS2Eとつなげるために作ったもので、これを介してS2Eと繋げれば、後はその先でS2E側でSILSかHILSか判断して、COMポート使ったり使わなかったりするという設計です。
CCSDS_SILS_SCI_IF.cはS2E関係なくC2A側が持っている機能で、C2Aが直接COMポートをいじるというもので、C2A-WINGSの直接接続というイメージです。

どちらもSILSという名前がついてしまっていますが、これはよくある

AOCSのSILS(S2E-C2Aがメイン)
CDHのSILS(C2A-WINGSがメイン)

の違いも関連しているので、名前も変えても良いかもですね。

S2E側としては、全てUART_SILS.cにしてCOMポート操作などはS2Eに任せて欲しいと思いつつ、S2Eを使わないC2A-WINGS通信の話などもあるので、難しいという感じですね。

200km · 2023-06-02T13:50:07Z

なので、姿勢系のworkflowを全てlinuxでもできるようにするという観点だと、

S2E-C2A　の　高速SILSはこの修正関係なくすでにできている
S2E-C2A-WINGS の　リアルタイムSILSはこの修正でできるようになる
HILS試験は、S2E側のCOMポート操作部を修正する必要がある

という状況で、本当は2と3を同じ枠組み(UART_SILS)で修正できたらよいなという感じで最初の質問をしたという感じです。

200km · 2023-06-02T13:56:40Z

CI回るようになりましたね。良かった。

200km · 2023-06-02T14:53:12Z

@seki-hiro CI通ったものの、修正内容的にoption(USE_SCI_COM "Use SCI_COM" ON)しないと影響がでないのでその上でWindowsでの動作確認はしてもらいたいです。
@conjikidow vMicroは全く関係ない場所なので、大丈夫だとは思いますが、ルール上ビルドチェックだけはお願いします。

conjikidow · 2023-06-03T00:00:15Z

背景や意図について理解しました，ありがとうございます。
CIについてもご対応ありがとうございます。

src/src_user/IfWrapper/SILS/sils_sci_if.h

200km · 2023-06-04T21:18:28Z

src/src_user/IfWrapper/SILS/sils_sci_if.h

+
+private:
+#ifdef WIN32
+  HANDLE myHComPort_;


[WANT] 全体的にdoxygenコメントをお願いします。

200km · 2023-06-04T21:19:23Z

src/src_user/IfWrapper/SILS/uart_sils_sci_if.h

-};
+// 最初だけ初期化して、プログラム終了時にポートを閉じるようにしたい
+#ifdef WIN32
+static SCIComPort SILS_SCI_IF_sci_com_uart_(11);


[NITS] ポート番号をいずれは外部ファイルからアクセスできるようにしたいです。TODOコメントでも良いので残しておいてください。

@200km
こちら，一旦 src/src_user/Settings/port_config.h で設定できるようにしました。
「ポート設定」という意味では一貫している一方，ここでは基本的にAOBCのポート設定をしているため分けるべきという考えもあります。
どのようにするのが良いか，ご意見をお伺いしたいです。

難しいですが、port_config.h とは別な気がするので、完全に別のファイルとしてsrc/src_user/Settings/sils_port_config.hなどを作るのが良い気がします。

ありがとうございます。そうさせていただきます。

修正しました。

src/src_user/IfWrapper/SILS/sils_sci_if.c

src/src_user/IfWrapper/SILS/sils_sci_if.h

src/src_user/IfWrapper/SILS/sils_sci_if.c

src/src_user/IfWrapper/SILS/sils_sci_if.h

src/src_user/IfWrapper/SILS/sils_sci_if.c

conjikidow · 2023-06-05T14:44:37Z

確認ありがとうございます。
後ほど見ます。

seki-hiro · 2023-06-19T07:33:14Z

CI通ったものの、修正内容的にoption(USE_SCI_COM "Use SCI_COM" ON)しないと影響がでないのでその上でWindowsでの動作確認はしてもらいたいです。

了解です。
@conjikidow 動作確認必要なタイミングで教えてください！

conjikidow · 2023-07-20T09:29:00Z

CIが通っていませんが，他（例）でも通っていないようなので修正を試みています。

200km · 2023-07-20T09:30:19Z

@conjikidow

CCSDS_SILS_SCI_IF.cも廃止するのが良い

念の為の確認ですが、下記issueにあるようなpytestを導入するときにも問題ないでしょうか？C2A本体やMOBC側のpytestの仕組みがわかっておらず少しだけ気になっています。

#28

@chutaro CDH系に聞くのが適切かもしれません。

conjikidow · 2023-07-20T09:33:02Z

@200km
確かにpytestでは必要になるかもしれないです。確認してみます。

200km · 2023-07-20T09:40:34Z

CIの件はここで議論した通り、今は全て通らなくても良いという状況です。

chutaro · 2023-07-20T10:03:57Z

CCSDS_SILS_SCI_IF.cも廃止するのが良い

はい、A/TOBCでは廃止してよいと思います

200km · 2023-07-20T10:14:57Z

@chutaro コメントありがとうございます。AOBCでpytestを導入するというときも必要ないということでしょうか？
導入する時の参考のためにもpytestの仕組み（特にC2A側がどうなっているか）教えてもらえると嬉しいです。

chutaro · 2023-07-20T10:28:25Z

c2a-aobcを動かしてwingsで通信できればpytestできるので、wingsとの通信で使っていない CCSDS_SILS_SCI_IF.c は廃止して問題ないです。mobcはccsdsラインを地上局用、uartラインをコンポ用にSILSで再現してますが、aobcはmobcとの通信のuartラインをSILSで再現すれば十分で、それを使えばpytestできます

200km · 2023-07-20T10:31:32Z

@chutaro ありがとうございます！

conjikidow · 2023-07-20T10:42:41Z

ローカルでも話しましたが，その方針で進めさせていただきます。

200km

port_configの部分だけ修正してもらえるとうれしいです。

src/src_user/IfWrapper/Sils/gpio_sils.c

200km · 2023-07-21T08:28:08Z

src/src_user/IfWrapper/SILS/uart_sils_sci_if.h

-};
+// 最初だけ初期化して、プログラム終了時にポートを閉じるようにしたい
+#ifdef WIN32
+static SCIComPort SILS_SCI_IF_sci_com_uart_(11);


難しいですが、port_config.h とは別な気がするので、完全に別のファイルとしてsrc/src_user/Settings/sils_port_config.hなどを作るのが良い気がします。

conjikidow · 2023-07-21T12:13:43Z

確認ありがとうございます。

@seki-hiro お時間のあるときに，Windows環境で，WINGSあり/なしそれぞれのSILSのテストをお願いできますでしょうか。
WINGSありの場合，c2a-aobc側のCMakeLists.txtでここのオプションをONにする必要があります。

seki-hiro · 2023-07-24T06:36:02Z

Windows + Visual Studio環境でもWINGSあり/なし両方で確認できました。

conjikidow · 2023-07-24T07:40:19Z

確認ありがとうございます。

conjikidow added priority::medium labels Jun 2, 2023

conjikidow requested review from 200km, chutaro, flap1, seki-hiro, suzuki-toshihir0 and t-hosonuma June 2, 2023 13:07

conjikidow self-assigned this Jun 2, 2023

conjikidow force-pushed the feature/support_linux branch from 99a0f4f to ff92fb6 Compare June 2, 2023 13:15

200km added the WINGS label Jun 2, 2023

200km reviewed Jun 4, 2023

View reviewed changes

github-actions bot removed patch labels Jul 14, 2023

Rename and refactor files

ed86817

conjikidow force-pushed the feature/support_linux branch from 6948707 to ed86817 Compare July 20, 2023 09:16

200km removed the request for review from a team July 20, 2023 09:23

Fix bug

78dc689

200km mentioned this pull request Jul 20, 2023

Pytestが実行できる体制を整える #28

Closed

2 tasks

200km self-requested a review July 20, 2023 11:53

conjikidow added 4 commits July 21, 2023 10:51

Delete ccsds_sils_if.*

bcd4cc5

Merge uart_sils_if.* into uart_sils.*

c4edc45

Add port config for sils

e194f2e

Add comments

4623612

conjikidow changed the title ~~WIP:Support Linux for SILS with WINGS~~ Support Linux for SILS with WINGS Jul 21, 2023

200km reviewed Jul 21, 2023

View reviewed changes

Move port config for sils to sils_port_config.h

3ae4083

conjikidow requested a review from 200km July 21, 2023 11:17

200km approved these changes Jul 21, 2023

View reviewed changes

seki-hiro approved these changes Jul 24, 2023

View reviewed changes

conjikidow merged commit 8d613ff into develop Jul 24, 2023

conjikidow deleted the feature/support_linux branch July 24, 2023 07:40

Support Linux for SILS with WINGS #76

Support Linux for SILS with WINGS #76

Conversation

conjikidow commented Jun 2, 2023 • edited by yukyukhello Loading

概要

Issue

詳細

検証結果

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

動作確認チェック (全てチェック)

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

影響範囲

補足

200km commented Jun 2, 2023

200km commented Jun 2, 2023

conjikidow commented Jun 2, 2023

200km commented Jun 2, 2023 • edited Loading

conjikidow commented Jun 2, 2023

200km commented Jun 2, 2023

200km commented Jun 2, 2023

conjikidow commented Jun 2, 2023

200km commented Jun 2, 2023 • edited Loading

200km commented Jun 2, 2023

200km commented Jun 2, 2023

200km commented Jun 2, 2023

conjikidow commented Jun 3, 2023

200km Jun 4, 2023

Choose a reason for hiding this comment

200km Jun 4, 2023

Choose a reason for hiding this comment

conjikidow Jul 21, 2023 • edited Loading

Choose a reason for hiding this comment

200km Jul 21, 2023

Choose a reason for hiding this comment

conjikidow Jul 21, 2023

Choose a reason for hiding this comment

conjikidow Jul 21, 2023

Choose a reason for hiding this comment

conjikidow commented Jun 5, 2023

seki-hiro commented Jun 19, 2023

conjikidow commented Jul 20, 2023

200km commented Jul 20, 2023 • edited Loading

conjikidow commented Jul 20, 2023

200km commented Jul 20, 2023 • edited Loading

chutaro commented Jul 20, 2023

200km commented Jul 20, 2023

chutaro commented Jul 20, 2023

200km commented Jul 20, 2023

conjikidow commented Jul 20, 2023

200km left a comment

Choose a reason for hiding this comment

200km Jul 21, 2023

Choose a reason for hiding this comment

conjikidow commented Jul 21, 2023 • edited Loading

seki-hiro commented Jul 24, 2023

conjikidow commented Jul 24, 2023

conjikidow commented Jun 2, 2023 •

edited by yukyukhello

Loading

200km commented Jun 2, 2023 •

edited

Loading

200km commented Jun 2, 2023 •

edited

Loading

conjikidow Jul 21, 2023 •

edited

Loading

200km commented Jul 20, 2023 •

edited

Loading

200km commented Jul 20, 2023 •

edited

Loading

conjikidow commented Jul 21, 2023 •

edited

Loading