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

Rename component: IfWrapper -> hal #21

Merged
merged 14 commits into from
Jul 24, 2023
Merged

Rename component: IfWrapper -> hal #21

merged 14 commits into from
Jul 24, 2023

Conversation

sksat
Copy link
Member

@sksat sksat commented Jul 21, 2023 •
edited
Loading

概要

IfWrapper を hal に rename する

Issue

詳細

IfWrapper は interface wrapper のことだが，これは C2A でしか用いられていない単語である上に，wrapper であることしかわからない．そのため，IfWrapper が追っていた主な責務である，ターゲットを問わないハードウェアレイヤの抽象化に着目し，組み込みソフトウェアの文脈で使われがちな HAL（Hardware Abstraction Layer）に rename する．

  • IfWrapper ディレクトリを hal に rename
  • IfWrapper の参照を hal に置換
  • src/src_user/IfWrapper の参照を src/src_user/hal に置換
  • C2A user 用の migration script を追加

検証結果

migration script を example user に対して実行して移行できればよし

影響範囲

  • IfWrapper という語がすべて hal になる
  • IfWrapper ディレクトリ以下のディレクトリが snake_case になる
    • src_user/IfWrapper/Common -> src_user/hal/common
    • src_user/IfWrapper/Sils -> src_user/hal/sils
  • src_core/IfWrapper/Common は confusing なので廃止（src_core/hal 直下に移動）
  • C2A user では，Script/migration/v4-rename-components.sh を実行して一気に置換して対応可能

@sksat sksat added documentation Improvements or additions to documentation enhancement New feature or request priority::high priorityg high tools labels Jul 21, 2023
@sksat sksat requested a review from meltingrabbit July 21, 2023 08:24
@sksat sksat self-assigned this Jul 21, 2023
@sksat
Copy link
Member Author

sksat commented Jul 21, 2023 •
edited
Loading

この rename によって src_user/IfWrapper/if_list.c の歪さが表出する．そもそもこの存在をどうにかしたいというのはありつつ，実態に即した場所と名前は src_user/Settings/DriverSuper/handler_registry.h などなので，これも変更する必要がある．

This was referenced Jul 21, 2023
Closed
Closed
@sksat
Copy link
Member Author

sksat commented Jul 21, 2023

IfWrapper/Common は何一つ IfWrapper の common ではないのでこれはこのタイミングで直すか．hal 以下ですらなくていいような気もしなくはないが，一旦 hal/i2c_utils.c ぐらいが妥当なところだろうか．

@sksat
Copy link
Member Author

sksat commented Jul 21, 2023 •
edited
Loading

IfWrapper/Common の問題は c2a-core と C2A user で使い方が微妙に異なること．c2a-core では i2c の（IfWrapper 実装に乗っかった）共通したユーティリティ関数の提供をしているのに対して，C2A user では複数ターゲット（実機・SILS）に共通した IfWrapper 実装の置き場になっている．後者については実機・SILS・共通部分というディレクトリの並列構造が存在するのでナシではないが，前者はあまり妥当ではないので，変える．

@meltingrabbit
Copy link
Member

雰囲気よさそう．userふくめた変更でき次第，再度レビューします．

@sksat sksat mentioned this pull request Jul 21, 2023
Merged
meltingrabbit
meltingrabbit approved these changes Jul 24, 2023
View reviewed changes
Copy link
Member

@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.

#24 と同タイミングでマージするとして，CIは通ってないがOK

@sksat
Copy link
Member Author

sksat commented Jul 24, 2023

では，マージ

@sksat sksat merged commit 0109b0d into develop Jul 24, 2023
7 of 23 checks passed
@sksat sksat deleted the feature/rename-ifwrapper branch July 24, 2023 07:12
sksat pushed a commit that referenced this pull request Sep 21, 2023
@meltingrabbit
Merge pull request #21 from ut-issl/feature/all_enum
6257ab6 
Enum をすべて引っかけられるように
@meltingrabbit meltingrabbit mentioned this pull request Nov 1, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meltingrabbit meltingrabbit meltingrabbit approved these changes

Assignees

@sksat sksat

Labels
documentation Improvements or additions to documentation enhancement New feature or request priority::high priorityg high tools
Projects
C2A public
Status: Done
Milestone
No milestone
Development

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