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

Split obc time config header #583

Merged
merged 4 commits into from
May 18, 2023
Merged

Conversation

sksat
Copy link
Collaborator

@sksat sksat commented May 18, 2023

概要

ObcTime の設定を obc_time.h から分割する

Issue

詳細

  • obc_time.h が C2A user に依存しなくなる
  • ObcTime のデフォルト設定は obc_time_config.h で定義するようになる

検証結果

CI が通ればよし

@sksat sksat self-assigned this May 18, 2023
@sksat
Copy link
Collaborator Author

sksat commented May 18, 2023

task dispatcher も使うのか.まあそりゃそうか.

@sksat sksat force-pushed the feature/split-obc-time-config-header branch from 1d04d43 to 4864b86 Compare May 18, 2023 10:17
@sksat
Copy link
Collaborator Author

sksat commented May 18, 2023

CI 通ったな

@sksat sksat requested a review from meltingrabbit May 18, 2023 10:25
@meltingrabbit meltingrabbit added the priority::medium priority medium label May 18, 2023
#define OBCT_MAX_CYCLE (0xfffffff0u) //!< 最大 cycle 数.つまり TI がいくつでオーバーフローするか

#include <src_user/Settings/System/obc_time_params.h>

typedef uint32_t cycle_t;
typedef uint32_t step_t;
Copy link
Collaborator

Choose a reason for hiding this comment

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

実はこのtypedefもconfigかも?(将来的にはここの型サイズもuser設定にしたい気がしている.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

メモリくうし,step_tはu8のuserがいてもいいじゃん,的な

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

いやー,それは意味合いがかなり変わってくるので,少なくとも今はやめたい

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK

認識合わせたいので1つ質問なんだけど,
define のパラメタは config だけど, typedef って何になるんだ?(Cのtypedef,コンテキストが深すぎて,どう捉えていいか謎い)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

どこまでがユーザ定義なのかを明確に分割したい,というのがそもそもの目的であって,将来的にユーザ定義にする かもしれない ものをユーザ定義のものであるかのように扱うべきではない(現状そうでないし,するとしてもそう変更する時にやるべき)

あと,ここがユーザ定義になってしまうと ObcTime が実質的にユーザ定義になってしまうので,この分離があんまりうれしくなくなる

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

型変わってしまうので個人的には typedef してるようなものをユーザ定義にはしたくない寄り.それでメモリ切り詰められるとかは確かにあるかもしれないけれど,発生する複雑性に対して得られるメリットがそこまであるかは結構疑問だし,アリだったとしても早すぎる最適化の類に思ってしまう.

Copy link
Collaborator

Choose a reason for hiding this comment

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

なるほどね,理解

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority::medium priority medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants