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

include/lib の構造(いつかやりたい) #434

Open
kenjihiranabe opened this issue Dec 4, 2024 · 17 comments
Open

include/lib の構造(いつかやりたい) #434

kenjihiranabe opened this issue Dec 4, 2024 · 17 comments

Comments

@kenjihiranabe
Copy link
Collaborator

複数サブプロジェクトのリポジトリのディレクトリ構成。

Image

  • リポジトリ直下に include と lib を置き、各サブプロジェクトは、そこに外部成果物(lib, include)を配置
  • 他プロジェクトはそこに include/link PATH のみを通す。(各サブプロジェクトディレクトリに直接パスを書かない)
  • さらに、他のユーザが外部からAPIを使おうと思った時、そこのみにアクセスすればできるように。

この構造は、UNIX の /usr/local/include /usr/local/lib と同じ。

@tmori
Copy link
Contributor

tmori commented Dec 16, 2024

まずは、制御側の冗長コードを外します。

@tmori
Copy link
Contributor

tmori commented Dec 16, 2024

冗長コード外しました。

0826d45

@tmori
Copy link
Contributor

tmori commented Dec 16, 2024

制御側の物理依存コードは以下あたりですね。

  1. hako::drone_physics::body_vector_from_ground()
  2. hako::drone_physics::EulerType
  3. hako::drone_physics::VectorType
  4. M_PI

M_PIは標準ライブラリ cmath をインクルードして解決するように変更します。

@kenjihiranabe
Copy link
Collaborator Author

M_PI は確か、森さんが _osdep.h ファイルを作って切り出したんだと思う。確か、ubuntu でビルド通らなかったのを直してくれたんだと思う。このヘッダーなければないで嬉しい。

@tmori
Copy link
Contributor

tmori commented Dec 16, 2024

M_PIですが、Windowsでもハマってます。。

ChatGPTに聞いたところこうしないとダメみたいっす。

#ifdef _WIN32
#define _USE_MATH_DEFINES
#endif
#include <cmath>

@kenjihiranabe
Copy link
Collaborator Author

コンパイル依存は結構小さいのね。
physics側で少し小さめのヘッダー/翻訳単位切ってもいいですよ。

最小リンク依存はどうでしょう。
最小の実行形式も、上記だけで成立するならそうしましょう。

最小実行形式が、他の関数も含むなら、どちらにしても全体依存なので、今の翻訳単位でライブラリ.aのリンクで良いと思う。

@tmori
Copy link
Contributor

tmori commented Dec 16, 2024

そうですね。

依存しているのは、hako::drone_physics::body_vector_from_ground() と付随するデータ型定義だけなので、そこだを切り出したヘッダとライブラリでリンクするのは手ですね。

@kenjihiranabe
Copy link
Collaborator Author

最小実行形式は何になる?

@tmori
Copy link
Contributor

tmori commented Dec 16, 2024

制御側が依存しているのは、以下のコードだけなので、ここだけを cコードとして切り出して、
ヘッダファイルから参照できれば良いと思っておりますが、回答なってますでしょうか?

/* for generic vectors. use the meaningful aliases below  */
VectorType body_vector_from_ground(
    const VectorType& ground,
    const EulerType& angle)
{
    using std::sin; using std::cos;
    const auto
        c_phi   = cos(angle.phi),   s_phi = sin(angle.phi),
        c_theta = cos(angle.theta), s_theta = sin(angle.theta),
        c_psi   = cos(angle.psi),   s_psi = sin(angle.psi);
    const auto [x_e, y_e, z_e] = ground;
    /*
     * See eq.(1.69), inverse of (1.124) in Nonami's book.
     * See also https://mtkbirdman.com/flight-dynamics-body-axes-system
     * for the transformation equations.
     */
    /*****************************************************************/  
    double
    x =   (c_theta * c_psi)                         * x_e
        + (c_theta * s_psi)                         * y_e
        - (s_theta)                                 * z_e;

    double
    y =   (s_phi * s_theta * c_psi - c_phi * s_psi) * x_e
        + (s_phi * s_theta * s_psi + c_phi * c_psi) * y_e
        + (s_phi * c_theta)                         * z_e;

    double
    z = (c_phi * s_theta * c_psi + s_phi * s_psi)   * x_e
        + (c_phi * s_theta * s_psi - s_phi * c_psi) * y_e
        + (c_phi * c_theta)                         * z_e;
    /*****************************************************************/  

    return {x, y, z};
}

@tmori
Copy link
Contributor

tmori commented Dec 16, 2024

VectoryTypeとEulerTypeは、drone_physics.hppに定義あるのですが、
データ型定義は別ヘッダにすると良いかもしれないなと思ってもいます。

@tmori
Copy link
Contributor

tmori commented Dec 17, 2024

自分の意見をまとめると、こうですかね。

  1. primitiveなデータ型定義ヘッダを作る
  2. drone_physics.hppは1のヘッダをinclude
  3. body_vector_from_ground()関数を定義するcコードをつくる
  4. body_vector_from_ground()関数宣言ヘッダを作る
  5. 制御側からは、4をincludeする

@kenjihiranabe
Copy link
Collaborator Author

最小実行形式( main を含むもの。テスト実行形式も含む)として physics 全体を含まないものがあれば、切り出すのはありかなと思っての質問です。どれだけコンパイル時依存を減らしても、実行時に全体依存してしまうなら、細かくしてもしょうがないかな、、、、と思うのです。

森さんの意見、了解しました。
ぼくの意見は、違っているので、書きます。

大きめのコードを(テンプレートでない限り).hpp に置くのは悪手(すべての場所で展開されていまうから)。これをやるとすると、そのヘッダは1つの .cpp にのみにインクルードすることだが、もしそうなら、その cpp に書くのがよい。そして、もしそうするのなら、最小実行形式で libdrone_physics.a/so をリンクしてしまえばいのでは?

あるいは、今の変換コードを含む .hpp を .hpp に宣言を残して実装を .cpp に追い出し、変換部分は drone_physics への呼び出しとしては?

@tmori
Copy link
Contributor

tmori commented Dec 17, 2024

対面で話した方が良さそうな気がしてきましたが、、、ちょっと絵にしてみました。

今、現状は、このようにしてビルドを通しました。

Image

自分の方としては、あまりこだわりはなくて、最小構成で切り出すのならという意見として書かせていただきました。

現状、body_vector_from_ground() は、ヘッダに直接展開したものを利用しているのではなくて、drone_physics.oをコンパイル/リンクして利用しております。

なので、以下のご懸念の点については問題ないと思いますが、どうでしょうか。

大きめのコードを(テンプレートでない限り).hpp に置くのは悪手(すべての場所で展開されていまうから)。これをやるとすると、そのヘッダは1つの .cpp にのみにインクルードすることだが、もしそうなら、その cpp に書くのがよい。

そして、もしそうするのなら、最小実行形式で libdrone_physics.a/so をリンクしてしまえばいのでは?

はい、それもありかなと思いましたが、、リポジトリ構成上同じところにおりますので、現状のままでも良いかもと思ったりしました。

@kenjihiranabe
Copy link
Collaborator Author

少し考えて、

  • drone_frames.hpp(新) … body <->ground の変換関数2つとタイプ(将来的にQuaternion版も) を作る。drone_frames.cpp も作る。
  • rotor_phisycs.hpp ... そのまま
  • body_phisycs.hpp … 現在の定義から drone_frames.hpp を抜く。

の3つに階層化して、下が上に依存するようにして、drone_frames のみを制御で使ってもらうことを可能にする。

理由。

  • 変換のみに関心があるユーザが将来でるかも(意味的なまとまり)
  • 確かに body_physics.cpp/hpp はちょっと大きすぎる
  • rotor_physics.hpp にも依存してしまっている

note: でも先の議論で、drone_frames は drone_control.so には含めず、main を持つ側がリンクする(ことができれば)それはそうしたい。

@kenjihiranabe
Copy link
Collaborator Author

kenjihiranabe commented Dec 26, 2024

$(PRJ_ROOT)/include, /lib に各サブモジュールのライブラリとパブリックヘッダを集める件ですが、「何がパブリックヘッダで何がパブリックライブラリか」を集中管理せず、各サブモジュールの MakeList.txt に任せてしまってはどうでしょう?
実験したらこんな感じでできました。physics の MakeList.txt です。

% make publish

すると include, lib にコピーします。最上位では、各サブディレクトリを make, make test, make publish を呼び出す。

set(HAKONIWA_PUBLIC_HEADER_DIR  ${CMAKE_SOURCE_DIR}/../include)
set(HAKONIWA_PUBLIC_LIB_DIR ${CMAKE_SOURCE_DIR}/../lib)
set(PUBLIC_HEADERS
    drone_frames.hpp
    drone_physics.hpp
    rotor_physics.hpp
    body_physics.hpp
    drone_physics_c.h
)

add_custom_target(
    publish
    COMMAND ${CMAKE_COMMAND} -E copy ${PUBLIC_HEADERS} "${HAKONIWA_PUBLIC_HEADER_DIR}"
    COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:drone_physics> $<TARGET_FILE:drone_physics_c> "${HAKONIWA_PUBLIC_LIB_DIR}"
)

@tmori
Copy link
Contributor

tmori commented Dec 26, 2024

おー、それです!

@tmori
Copy link
Contributor

tmori commented Dec 30, 2024

やってみました。

https://github.com/toppers/hakoniwa-drone-core/issues/25

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

No branches or pull requests

2 participants