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.

Sign up for GitHub

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

Jump to bottom

GPS時刻スパイクフィルタの追加 #237

Merged
merged 13 commits into from
Oct 13, 2023
Merged

GPS時刻スパイクフィルタの追加 #237

merged 13 commits into from
Oct 13, 2023

Conversation

t-hosonuma
Copy link
Contributor

@t-hosonuma t-hosonuma commented Oct 8, 2023 •
edited
Loading

Issue

詳細

GPS時刻に関するスパイクフィルタをアプリoem7600_filterに追加．
上記に伴い，関連するライブラリ等を更新．

検証結果

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

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

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

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

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

image

補足

NA

@t-hosonuma t-hosonuma added the 🐬 minor update Minor update label Oct 8, 2023
@t-hosonuma t-hosonuma added this to the v8.0.0 Major update milestone Oct 8, 2023
@t-hosonuma t-hosonuma requested a review from 200km October 8, 2023 08:27
@t-hosonuma t-hosonuma self-assigned this Oct 8, 2023
@t-hosonuma t-hosonuma requested review from sksat and a team as code owners October 8, 2023 08:27
@t-hosonuma t-hosonuma requested review from suzuki-toshihir0 and conjikidow and removed request for a team October 8, 2023 08:27
@t-hosonuma t-hosonuma changed the title WIP:GPS時刻スパイクフィルタの追加 GPS時刻スパイクフィルタの追加 Oct 10, 2023
@200km 200km added the ✈️ priority::medium priority medium label Oct 10, 2023
200km
200km reviewed Oct 11, 2023
View reviewed changes
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.

命名についての細かなコメントをつけさせてもらいました。

src/src_user/Applications/UserDefined/AOCS/HardwareDependent/SensorFilters/oem7600_filter.c Outdated Show resolved Hide resolved
src/src_user/Applications/UserDefined/AOCS/HardwareDependent/SensorFilters/oem7600_filter.h Outdated Show resolved Hide resolved
src/src_user/Applications/UserDefined/AOCS/HardwareDependent/SensorFilters/oem7600_filter.c Outdated Show resolved Hide resolved
src/src_user/Applications/UserDefined/AOCS/HardwareDependent/SensorFilters/oem7600_filter.h Outdated Show resolved Hide resolved
src/src_user/Library/SignalProcess/spike_filter.c Outdated Show resolved Hide resolved
src/src_user/Library/TimeSystem/gps_time.c Outdated Show resolved Hide resolved
src/src_user/Library/TimeSystem/gps_time.c Outdated Show resolved Hide resolved
@200km
Copy link
Member

200km commented Oct 12, 2023

oem関連で出ているwarningsも解消していただけると嬉しいです。
https://github.com/ut-issl/c2a-aobc/actions/runs/6478723664/job/17590965759?pr=237#step:12:207
や
https://github.com/ut-issl/c2a-aobc/actions/runs/6478723664/job/17590965855?pr=237#step:11:200

などです。

@t-hosonuma
Copy link
Contributor Author

これは別PRとさせて頂きたいです．

oem関連で出ているwarningsも解消していただけると嬉しいです。 https://github.com/ut-issl/c2a-aobc/actions/runs/6478723664/job/17590965759?pr=237#step:12:207 や https://github.com/ut-issl/c2a-aobc/actions/runs/6478723664/job/17590965855?pr=237#step:11:200

などです。

@200km
Copy link
Member

200km commented Oct 12, 2023

これは別PRとさせて頂きたいです

今回の実装に関係ない部分は次PRでも良いですが、下記の部分などは今回追加実装分に出ているwarningだと思います。そのようなwarningに関してはこのPRで対応すべきだと思いますので、お願いします。

https://github.com/ut-issl/c2a-aobc/actions/runs/6498394983/job/17649635619?pr=237#step:11:200

200km
200km reviewed Oct 12, 2023
View reviewed changes
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.

タイポなど細かい部分が多くて申し訳ないですが、過去にタイポを一掃する作業を行った時にとても大変だったので、一つ一つのレビューの中で丁寧に対応していきたいと思っていますので、よろしくお願いします。

src/src_user/Applications/UserDefined/AOCS/HardwareDependent/SensorFilters/oem7600_filter.c Outdated Show resolved Hide resolved
src/src_user/Applications/UserDefined/AOCS/HardwareDependent/SensorFilters/oem7600_filter.c Outdated

static Oem7600Filter oem7600_filter_;
const Oem7600Filter* const oem7600_filter = &oem7600_filter_;

static SpikeFilter APP_OEM7600_FILTER_position_spike_[PHYSICAL_CONST_THREE_DIM]; //!< スパイク除去フィルタ for 衛星位置
static SpikeFilter APP_OEM7600_FILTER_velocity_spike_[PHYSICAL_CONST_THREE_DIM]; //!< スパイク除去フィルタ for 衛星速度
static SpikeFilter APP_OEM7600_FILTER_gps_time_total_sec_spike_; //!< スパイク除去フィルタ for gpstime
Copy link
Member

@200km 200km Oct 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

[NITS] 細かくてすみませんが、コメントの中のものもGPS timeなど単語として正しくしてもらえると嬉しいです。（このような細かなミスが、エディタが持つタイポ探知機能に引っかかり、逆に真のタイポが埋もれて見つけづらくなるためです。）

Copy link
Member

Choose a reason for hiding this comment

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

こちらのコメント内のgpstimeが修正されればマージして良いと思いますので、Approveします。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

修正致しましたのでマージします

Copy link
Member

Choose a reason for hiding this comment

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

ありがとうございます。

src/src_user/Applications/UserDefined/AOCS/HardwareDependent/SensorFilters/oem7600_filter.c Outdated Show resolved Hide resolved
src/src_user/Library/TimeSystem/gps_time.c Outdated Show resolved Hide resolved
src/src_user/Library/physical_constants.h Outdated Show resolved Hide resolved
@200km 200km self-requested a review October 13, 2023 04:49
200km
200km approved these changes Oct 13, 2023
View reviewed changes
src/src_user/Applications/UserDefined/AOCS/HardwareDependent/SensorFilters/oem7600_filter.c Outdated

static Oem7600Filter oem7600_filter_;
const Oem7600Filter* const oem7600_filter = &oem7600_filter_;

static SpikeFilter APP_OEM7600_FILTER_position_spike_[PHYSICAL_CONST_THREE_DIM]; //!< スパイク除去フィルタ for 衛星位置
static SpikeFilter APP_OEM7600_FILTER_velocity_spike_[PHYSICAL_CONST_THREE_DIM]; //!< スパイク除去フィルタ for 衛星速度
static SpikeFilter APP_OEM7600_FILTER_gps_time_total_sec_spike_; //!< スパイク除去フィルタ for gpstime
Copy link
Member

Choose a reason for hiding this comment

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

こちらのコメント内のgpstimeが修正されればマージして良いと思いますので、Approveします。

@t-hosonuma t-hosonuma merged commit db454e9 into develop Oct 13, 2023
9 of 11 checks passed
@t-hosonuma t-hosonuma deleted the feature/add_gpstime_spike_filte branch October 13, 2023 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@200km 200km 200km approved these changes

@sksat sksat Awaiting requested review from sksat sksat is a code owner

@suzuki-toshihir0 suzuki-toshihir0 Awaiting requested review from suzuki-toshihir0 suzuki-toshihir0 is a code owner automatically assigned from ut-issl/aocs

@conjikidow conjikidow Awaiting requested review from conjikidow conjikidow is a code owner automatically assigned from ut-issl/aocs

Assignees

@t-hosonuma t-hosonuma

Labels
✈️ priority::medium priority medium 🐬 minor update Minor update
Projects
None yet
Milestone
v8.0.0 Major update
Development

Successfully merging this pull request may close these issues.
2 participants
@t-hosonuma @200km