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

Command パケット周辺の簡易リファクタリング #287

Merged
merged 31 commits into from
Mar 22, 2022
Merged

Command パケット周辺の簡易リファクタリング #287

merged 31 commits into from
Mar 22, 2022

Conversation

yngyu
Copy link
Contributor

@yngyu yngyu commented Mar 8, 2022 •
edited
Loading

概要

Command パケット周辺の簡易リファクタリング

Issue

詳細

  • パディングを考慮していなかったので構造体通りだと場所がずれていたのを修正した
  • 上部にパディングが発生すると嫌なのでパケットの定義は uint8_t の配列にした (末尾に発生する可能性はある)
    • packed 属性の使用はやめた
    • body 部分の取得は関数経由でポインタ取得にした
  • COP-1 制御周りを少し書き直した
  • GS Driver 周りの動作に影響しないバグを直した

検証結果

手元でテストを一通り通したが、AD, BC コマンドについては確認されていない

影響範囲

大

@yngyu yngyu added the priority::high priorityg high label Mar 8, 2022
@yngyu yngyu requested review from chutaro and meltingrabbit March 8, 2022 15:39
@yngyu yngyu self-assigned this Mar 8, 2022
@yngyu yngyu force-pushed the feature/fix_cmd_packet_struct branch from 2a6a7bc to ee7e7bc Compare March 9, 2022 06:32
@yngyu yngyu changed the title Draft: Command パケット周辺の簡易リファクタリング Command パケット周辺の簡易リファクタリング Mar 9, 2022
meltingrabbit
meltingrabbit reviewed Mar 9, 2022
View reviewed changes
Copy link
Collaborator

@meltingrabbit meltingrabbit left a comment

Choose a reason for hiding this comment

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

ちょっと一旦GSまでで一区切り

Examples/minimum_user_for_s2e/src/src_user/Drivers/Com/gs.c Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/Drivers/Com/gs.h Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/Drivers/Com/gs.c Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/TlmCmd/Ccsds/tc_transfer_frame.c Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/Drivers/Com/gs.c Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/Drivers/Com/gs_validate.h Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/Drivers/Com/gs.c Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/Drivers/Com/gs.c Show resolved Hide resolved
@meltingrabbit
Copy link
Collaborator

meltingrabbit commented Mar 9, 2022 •
edited
Loading

これ，いい感じになってきてる（偉業）し，どこかでcore移植も目に見えてきたな．

@meltingrabbit
Copy link
Collaborator

レビューしながら，ちょっと少しだけ細かいcommitしてますので，取り込みお願いします．

@meltingrabbit meltingrabbit force-pushed the feature/fix_cmd_packet_struct branch from 1cebff3 to a75f750 Compare March 9, 2022 11:36
@meltingrabbit meltingrabbit force-pushed the feature/fix_cmd_packet_struct branch from a75f750 to 1a8243e Compare March 9, 2022 11:39
@meltingrabbit

This comment was marked as resolved.

Sign in to view
@meltingrabbit

This comment was marked as resolved.

Sign in to view
@meltingrabbit
Copy link
Collaborator

meltingrabbit commented Mar 9, 2022 •
edited
Loading

[IMO]
GS_set_farm_pw が gs_validate.c にいるのどうなんだろうって思ったけど，まあ検証のための値だからいいのか？

なんか，COPは別ファイルに切り出したい？（それこそCCSDSの下とかに）

重いので別PRで良さそう

meltingrabbit
meltingrabbit reviewed Mar 9, 2022
View reviewed changes
Copy link
Collaborator

@meltingrabbit meltingrabbit left a comment

Choose a reason for hiding this comment

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

gs_validate.c についてのレビュー．

COP周りのロジックはまだおってないです．表面的なレビューだけしてます．

Examples/minimum_user_for_s2e/src/src_user/Drivers/Com/gs_validate.c Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/Drivers/Com/gs_validate.c Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/Drivers/Com/gs_validate.c Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/TlmCmd/Ccsds/tc_transfer_frame.c Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/TlmCmd/Ccsds/tc_transfer_frame.c Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/Drivers/Com/gs_validate.c Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/Drivers/Com/gs_validate.c Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/Drivers/Com/gs_validate.c Outdated Show resolved Hide resolved
meltingrabbit
meltingrabbit reviewed Mar 9, 2022
View reviewed changes
Examples/minimum_user_for_s2e/src/src_user/TlmCmd/Ccsds/tc_segment.h Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/TlmCmd/Ccsds/tc_segment.h Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/TlmCmd/Ccsds/tc_segment.h Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/TlmCmd/Ccsds/tc_transfer_frame.c Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/TlmCmd/Ccsds/tc_segment.c Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/TlmCmd/Ccsds/tc_segment.c Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/TlmCmd/Ccsds/tc_transfer_frame.h Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/TlmCmd/Ccsds/tc_transfer_frame.h Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/TlmCmd/Ccsds/tc_transfer_frame.c Outdated Show resolved Hide resolved
Examples/minimum_user_for_s2e/src/src_user/TlmCmd/Ccsds/tc_transfer_frame.c Outdated Show resolved Hide resolved
@yngyu yngyu force-pushed the feature/fix_cmd_packet_struct branch from 48e1025 to b880a5a Compare March 10, 2022 15:12
@yngyu

This comment was marked as resolved.

Sign in to view
@yngyu yngyu force-pushed the feature/fix_cmd_packet_struct branch from 1999f34 to 0e9d43e Compare March 22, 2022 02:38
meltingrabbit
meltingrabbit approved these changes Mar 22, 2022
View reviewed changes
Copy link
Collaborator

@meltingrabbit meltingrabbit left a comment

Choose a reason for hiding this comment

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

よさそう．お疲れ様です～

@yngyu
Copy link
Contributor Author

yngyu commented Mar 22, 2022

テスト全部通ったのでマージします

@yngyu yngyu merged commit 8f91e97 into develop Mar 22, 2022
@yngyu yngyu deleted the feature/fix_cmd_packet_struct branch March 22, 2022 03:24
@meltingrabbit meltingrabbit mentioned this pull request Mar 24, 2022
Open
@meltingrabbit meltingrabbit mentioned this pull request Jun 28, 2022
Merged
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

@meltingrabbit meltingrabbit meltingrabbit approved these changes

@chutaro chutaro Awaiting requested review from chutaro

Assignees

@yngyu yngyu

Labels
priority::high priorityg high
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yngyu @meltingrabbit