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

GPS時刻関連の更新 #79

Merged
merged 20 commits into from
Aug 10, 2023
Merged

GPS時刻関連の更新 #79

merged 20 commits into from
Aug 10, 2023

Conversation

t-hosonuma
Copy link
Contributor

@t-hosonuma t-hosonuma commented Jun 3, 2023

概要

GPS時刻関連の更新

Issue

詳細

https://github.com/ut-issl/c2a-aobc/issues/19のうち,1項目目と5項目目への対応

  • GPS時刻構造体をgps_time_of_week.hにて定義
  • 構造体でないgps時刻を参照していた箇所を上記構造体で全て置換え
  • time_space_calculator.cにて実施しているGPS時刻からのJday更新について,Jday更新処理がGPSのTLM更新頻度よりも高頻度な場合への対応を追加.

検証結果

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

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

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

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

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

  • 改修後のJday更新頻度が10Hz(10Hzで見たインクリメント値が秒換算で0.1)になっていることをS2E上で確認した.
  • 詳細や数値データ等は検証フォルダにupload

影響範囲

Jdayを利用している各機能へ影響

補足

N/A

注意

  • 6U AOCS team Projects への紐付けを行うこと
  • Assignees を自分に設定すること
  • Reviewers を設定すること
  • priority ラベルやmajor/minor/patch updateラベルを付けること

@t-hosonuma t-hosonuma added this to the 開発仮目標1 milestone Jun 3, 2023
@t-hosonuma t-hosonuma self-assigned this Jun 3, 2023
@t-hosonuma t-hosonuma changed the title WIP: GPS時刻関連の更新 WIP:GPS時刻関連の更新 Jun 3, 2023
@t-hosonuma t-hosonuma changed the title WIP:GPS時刻関連の更新 draft:GPS時刻関連の更新 Jun 3, 2023
@t-hosonuma t-hosonuma changed the title draft:GPS時刻関連の更新 WIP:GPS時刻関連の更新 Jun 3, 2023
@t-hosonuma t-hosonuma requested a review from 200km June 3, 2023 09:55
@t-hosonuma
Copy link
Contributor Author

@200km
デバッグは未実施ですが,実装自体は済んでいますので,一旦こちらでレビューをお願いいたします.

Copy link
Member

@200km 200km left a comment

Choose a reason for hiding this comment

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

コメント付けました。よろしくお願いいたします。

C2A_ISSL6U_AOBC.vcxproj Outdated Show resolved Hide resolved
src/src_user/Library/gps_time_of_week.c Outdated Show resolved Hide resolved
src/src_user/Library/gps_time_of_week.h Outdated Show resolved Hide resolved
src/src_user/Library/gps_time_of_week.h Outdated Show resolved Hide resolved
src/src_user/Library/gps_time_of_week.h Outdated Show resolved Hide resolved
src/src_user/Library/time_space.c Show resolved Hide resolved
@t-hosonuma
Copy link
Contributor Author

t-hosonuma commented Jun 9, 2023

@200km
一通りコメント対応が済んだように思います.
これで問題なければconfilct解消したのちvmicroでのビルド確認とs2e上での動作確認をします.
(何か,差分とコメントの対応が分かりにくいですね…,gitlabだと変更後のリンクがコメント欄に表示されていた様な…?
コメント欄に差分内容を記載した方が良かったりしますか…?)

@200km
Copy link
Member

200km commented Jun 9, 2023

コメント欄に差分内容を記載した方が良かったりしますか…

僕自身は手元で別に差分を見ているので必要ないです。大幅な変更や指摘内容とも違う方針変更についてはコメントに書いてもらえるとより注意してみれるのでありがたいです。

src/src_user/Library/time_system/gps_time.h Outdated Show resolved Hide resolved
src/src_user/Library/time_space.c Outdated Show resolved Hide resolved
src/src_user/Library/time_space.c Outdated Show resolved Hide resolved
src/src_user/Library/time_space.c Outdated Show resolved Hide resolved
@@ -43,10 +43,16 @@ static void APP_TIME_SPACE_CALC_init_(void)

static void APP_TIME_SPACE_CALC_exec_(void)
Copy link
Member

Choose a reason for hiding this comment

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

[Q] ここのアルゴリズムを軽く説明していただけると、レビューしやすいかもしれません。

Copy link
Contributor Author

@t-hosonuma t-hosonuma Jun 12, 2023

Choose a reason for hiding this comment

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

大きな方針は, 以前と変わらず,下記の様な方針です.

  • GPSテレメの更新があったらAPP_TIME_SPACE_CALC_update_current_jday_ref_にてGPSテレメ内の時刻をjdayに変換することでjdayを更新
  • GPSテレメの更新がなければAPP_TIME_SPACE_CALC_propagate_current_jday_ref_でAOCSマネージャ内のJdayをインクリメントすることでJdayを伝播

なのですが,GPSテレメの更新をどう判定するかという点を改修しており,その結果,見た目がややこしくなっていると思います.
具体的には,下記の様に修正しています.

  • 以前は,gps_visibilityが可視であれば更新アリとしていたが,GPSテレメの更新頻度よりもこの関数のcall頻度が高い場合にはこの条件だけでは判定が不十分だった(更新されていない同一のGPS時刻でjday換算を複数回実施しうる)
  • そのため,上記に加えて,Driver側がGPSテレメを受信した時刻であるaocs_manager->obct_gps_time_obsを見る様にし,最後にGPSテレメベースのjday変換を行った際に使ったGPSテレメ(にタグ付けされていた)受信時刻に対し,今回jday変換に使おうとしているGPSテレメ(にタグ付けされている)受信時刻が更新されているかどうかも確認する様にした
  • 上記の確認結果とgps_visibilityが可視であることのANDが成立する時だけ,GPSテレメを使ったjday変換を行うようにした.(ので,GPS時刻が更新されていない場合は,jday換算は行われず,AOBCの自走でjdayが伝播される様になっている見込み)

Copy link
Member

Choose a reason for hiding this comment

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

アルゴリズムありがとうございます。あとは、そのような動作になっているか動作確認を乗せていただいて判断したいと思います。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

この後の定例で少し話せればと思いますが,S2E上で上記の様な動作になっていることは確認できました.
(1HzでくるGPSからの時刻情報を,AOBCの自走クロックを使って10Hzで補間することで,GPSが可視の場合でも10HzでJdayが更新される)
ただ,GPSが常に非可視で,完全にAOBCだけで自走している場合と比べ,GPSが可視の場合だと,少しJdayの変化量がガタついている様に見えます.S2E上でC2A_AOCSを動かす際に,GPS時刻の基になっているS2E側の時間ステップがインクリメントされるタイミングと,C2A側のmaster_cycleをインクリメントするタイミングに何かしらズレがあるならばこういうことも起こり得るかと思うのですが,S2E上でこの2つのインクリメントタイミングは同期させていないと思って良いでしょうか…?
無題

Copy link
Member

Choose a reason for hiding this comment

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

@t-hosonuma ミーティングで話しましたが、S2Eでは時間は一つのカウントアップだけに紐づいているのでばらつきがあるというのはなさそうです。そのあと少し考えてみたのですが、もしかするとfloat/double変換のばらつきとかそういう部分の方が可能性があるかもしれませんが、心当たりありますか?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

有難うございます,

float/double変換のばらつき

そうですね,数値の桁の精度の話はありそうな気がしています(Jdayを0.1秒単位で比較するのがそもそもシビアなので…)
もう少し(検証のやり方そのものも含め)確認してみます.

Copy link
Contributor Author

@t-hosonuma t-hosonuma Aug 9, 2023

Choose a reason for hiding this comment

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

このバタつきの原因は,(1) Excel上での計算の桁落ちすること,及び,(2) S2E上のGNSSモデルが出力するdouble型GPS時刻をuint32型のGPSレシーバコンポ出力模擬(msec時刻)に変換した際に,int型への切り詰めによってmsec以下の精度が失われることの様です.(搭載側というよりS2E_6Uの機器モデル側が原因)
詳細は検証結果に記載します.

Copy link
Member

Choose a reason for hiding this comment

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

@t-hosonuma 原因調査ありがとうございます。S2E_AOBC側で修正が必要とのことなので、そちらでissueを切っておきます。

Copy link
Member

Choose a reason for hiding this comment

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

@200km 200km added 🐛 bug Something isn't working ✈️ priority::medium priority medium 🐬 minor update Minor update labels Jul 14, 2023
@t-hosonuma t-hosonuma requested review from sksat and a team as code owners August 9, 2023 01:04
@t-hosonuma t-hosonuma requested a review from 200km August 9, 2023 07:15
@t-hosonuma
Copy link
Contributor Author

一通り,コメント対応及び検証が済みました

@200km 200km removed request for a team and chutaro August 9, 2023 08:52
src/src_user/Library/time_space.c Outdated Show resolved Hide resolved
src/src_user/Library/time_space.c Show resolved Hide resolved
src/src_user/Library/time_system/gps_time.h Outdated Show resolved Hide resolved
@t-hosonuma t-hosonuma changed the title WIP:GPS時刻関連の更新 GPS時刻関連の更新 Aug 10, 2023
@200km 200km self-requested a review August 10, 2023 08:31
Copy link
Member

@200km 200km left a comment

Choose a reason for hiding this comment

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

修正確認したのでapproveします。

@t-hosonuma
Copy link
Contributor Author

ごめんなさい,これMergeボタンはこちらで押す形態でしたっけ…?

@200km
Copy link
Member

200km commented Aug 10, 2023

はい。マージ押してください!

@t-hosonuma t-hosonuma merged commit 1439762 into develop Aug 10, 2023
7 of 10 checks passed
@t-hosonuma t-hosonuma deleted the feature/gps_refactor branch August 10, 2023 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✈️ priority::medium priority medium 🐛 bug Something isn't working 🐬 minor update Minor update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants