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

Modify Calculation Function of Ground Image Position in Telescope #641

Open
wants to merge 42 commits into
base: develop
Choose a base branch
from

Conversation

Hiro-0110
Copy link
Member

@Hiro-0110 Hiro-0110 commented May 7, 2024

Related issues

Mention any issues that this pull request is related (e.g., #1). Consider using the development field if you want to close the issue automatically when this pull request is merged.

Description

撮像時間幅のの開始時刻付近, 中央時刻付近, 終了時刻付近における, センサ中央に対応する地表点, センサy最大位置に対応する地表点, センサy最小位置に対応する地表点の動きを計算してグラフ化するようにした

Test results

240605_013615_ground_position_1st
240605_013615_ground_position_2nd
240605_013615_ground_position_3rd

Impact

Describe the scope of influence of the changes, e.g., The behavior of feature ** changes.

Supplementary information

Provide any supplementary information.

@Hiro-0110 Hiro-0110 requested review from sksat and a team as code owners May 7, 2024 07:36
@Hiro-0110 Hiro-0110 requested review from 200km, seki-hiro, suzuki-toshihir0, t-hosonuma and conjikidow and removed request for a team May 7, 2024 07:36
@Hiro-0110 Hiro-0110 self-assigned this May 7, 2024
@Hiro-0110 Hiro-0110 changed the title WIP : Modify ObserveGroundPositionDeviation function WIP : Modify Calculation Function of Ground Image Position in Telescope May 24, 2024
@Hiro-0110 Hiro-0110 changed the title WIP : Modify Calculation Function of Ground Image Position in Telescope Modify Calculation Function of Ground Image Position in Telescope Jun 4, 2024
Copy link
Member

@seki-hiro seki-hiro left a comment

Choose a reason for hiding this comment

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

計算時間の求め方以外は、アルゴリズム面に問題はないと思います。単位が実態と異なる変数や、少し読みにくい部分があるので、修正してもらえると嬉しいです。

src/components/real/mission/telescope.cpp Outdated Show resolved Hide resolved
src/components/real/mission/telescope.cpp Show resolved Hide resolved
src/components/real/mission/telescope.cpp Show resolved Hide resolved
src/components/real/mission/telescope.cpp Outdated Show resolved Hide resolved
src/components/real/mission/telescope.cpp Outdated Show resolved Hide resolved
src/components/real/mission/telescope.cpp Outdated Show resolved Hide resolved
src/components/real/mission/telescope.cpp Outdated Show resolved Hide resolved
src/components/real/mission/telescope.hpp Outdated Show resolved Hide resolved
src/components/real/mission/telescope.hpp Outdated Show resolved Hide resolved
@200km
Copy link
Member

200km commented Jun 7, 2024

[Q] s2e-aobcの方にも似たコメントをつけましたが、こちら元々一般性の高いtelescopeに対して、あまり一般的でないTDIの機能を入れようとしているように見えます。

本来はこのtelescopeをベースにTDI用のより特殊なtelescope_with_tdiなどを別途作るべきではないでしょうか?

@seki-hiro
Copy link
Member

@Hiro-0110 修正ありがとうございます。アルゴリズム面は問題ないと思います。

実装場所はここではなくなるということだったので、誤ってマージすることがないように、approveはしないでおきますね。

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

Successfully merging this pull request may close these issues.

3 participants