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

Common Packet のディレクトリ変更 #310

Merged
merged 7 commits into from
Apr 3, 2024

Conversation

meltingrabbit
Copy link
Member

@meltingrabbit meltingrabbit commented Feb 13, 2024

概要

「CTCP, CTP, CCP のデフォルト実装を core で用意する」ための準備段階として,Common Packet のディレクトリ変更する.

Issue

詳細

  • CTCP, CTP, CCP のデフォルト実装を core で用意する #305 (comment) 参照
  • common packet は,その実体は Space Packet (SP) でもその他(例えば IP packet)でもよく,現在はそのヘッダのみ core にあり,実装は user にある.
  • 一方で,各 user でその実装を個別で管理するのは効率が悪く,core にデフォルト実装を格納することとした(ひとまずは SP のみ)
  • それにともない,ディレクトリを整理し, common packet のディレクトリをきる(← これがこのPR)
  • このあと,このディレクトリ配下に SP の実装を配置する予定.
    • \tlm_cmd\common_packet\space_packet のようになる予定

検証結果

CI が通れば OK

影響範囲

  • Common Packet の include path が変わる.

備考

@meltingrabbit meltingrabbit changed the title WIP WIP: Common Packet のディレクトリ変更 Feb 13, 2024
@meltingrabbit
Copy link
Member Author

まじか~. S2E に対しても braking かぁ

https://github.com/arkedge/c2a-core/actions/runs/7882167114/job/21506928888?pr=310

D:\a\c2a-core\c2a-core\s2e-core\src\components\real\communication\wings_command_sender_to_c2a.cpp(20,10): fatal  error C1083: Cannot open include file: 'src_core/tlm_cmd/common_cmd_packet_util.h': No such file or directory [D:\a\c2a-core\c2a-core\s2e-user\s2e_core\components\COMPONENT.vcxproj]

@meltingrabbit meltingrabbit self-assigned this Feb 13, 2024
@meltingrabbit meltingrabbit changed the title WIP: Common Packet のディレクトリ変更 Common Packet のディレクトリ変更 Feb 13, 2024
@meltingrabbit meltingrabbit marked this pull request as ready for review February 13, 2024 05:34
@meltingrabbit meltingrabbit added enhancement New feature or request priority::high priorityg high labels Feb 13, 2024
@meltingrabbit
Copy link
Member Author

@sksat S2E Core に対して breaking となってしまったんですが,どういう手順で進めればよいですか?(最近 S2E 触っておらず)

@meltingrabbit
Copy link
Member Author

@sksat
Copy link
Member

sksat commented Feb 13, 2024

これは S2E に最近生えた experimental な機能なので,互換性破壊してもよいです.パッチ送るのはこっちの変更が確定してからでお願いします(向こうも混乱するので + パッチ送る時も僕のレビューを必ず通してください).

@meltingrabbit
Copy link
Member Author

@sksat c2a側のディレクトリ変更はこれで良いと思ってるので

ut-issl/s2e-core#610

を出しました.確認ください.

@meltingrabbit

This comment was marked as outdated.

@meltingrabbit meltingrabbit force-pushed the feature/add_mobc_subobc_build_option branch from 7ef6111 to 0b8e8e9 Compare March 13, 2024 05:45
@meltingrabbit meltingrabbit force-pushed the feature/add_defalt_impl_for_ctcp branch from 86cc3ce to d94a334 Compare March 13, 2024 05:45
@meltingrabbit meltingrabbit mentioned this pull request Mar 13, 2024
6 tasks
@meltingrabbit meltingrabbit force-pushed the feature/add_mobc_subobc_build_option branch from 0b8e8e9 to 9a7f6bf Compare March 28, 2024 05:42
@meltingrabbit meltingrabbit force-pushed the feature/add_defalt_impl_for_ctcp branch from d94a334 to 622f8c5 Compare March 28, 2024 05:43
@meltingrabbit meltingrabbit force-pushed the feature/add_mobc_subobc_build_option branch from 4ed678d to 4130dff Compare March 29, 2024 03:48
@meltingrabbit meltingrabbit force-pushed the feature/add_defalt_impl_for_ctcp branch from 622f8c5 to 878e4b4 Compare March 29, 2024 03:49
@meltingrabbit meltingrabbit force-pushed the feature/add_mobc_subobc_build_option branch from 4130dff to a66034e Compare March 29, 2024 04:30
@meltingrabbit meltingrabbit force-pushed the feature/add_defalt_impl_for_ctcp branch from 878e4b4 to c8915c4 Compare March 29, 2024 04:35
Base automatically changed from feature/add_mobc_subobc_build_option to main March 29, 2024 04:40
@meltingrabbit meltingrabbit merged commit b510d9d into main Apr 3, 2024
37 checks passed
@meltingrabbit meltingrabbit deleted the feature/add_defalt_impl_for_ctcp branch April 3, 2024 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority::high priorityg high
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants