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

Add moon rotation #511

Merged
merged 14 commits into from
Oct 12, 2023
Merged

Add moon rotation #511

merged 14 commits into from
Oct 12, 2023

Conversation

200km
Copy link
Member

@200km 200km commented Oct 9, 2023

Related issues

#164

Description

I added the moon rotation definition.

Test results

I confirmed that the calculation result of Simple mode is similar with the result of IauMoon mode, which is calculated by SPICE.

image

Impact

Minor update

Supplementary information

NA

@200km 200km added priority::medium priority medium environment environment calculation minor update add functionality in a backwards compatible manner labels Oct 9, 2023
@200km 200km added this to the Major Update for v7.0.0 milestone Oct 9, 2023
@200km 200km self-assigned this Oct 9, 2023
@200km 200km requested review from sksat and a team as code owners October 9, 2023 14:54
@200km 200km requested review from suzuki-toshihir0, t-hosonuma and conjikidow and removed request for a team October 9, 2023 14:54
@200km 200km linked an issue Oct 9, 2023 that may be closed by this pull request
Base automatically changed from feature/refactor-celes-rotation to develop October 11, 2023 08:21
}

libra::Matrix<3, 3> CalcDcmMeanEarthToPrincipalAxis() {
const double theta_x_rad = 0.285 * libra::arcsec_to_rad;

Choose a reason for hiding this comment

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

Could you please briefly explain the meaning of the magic numbers (ex,. 0.285) by adding comment lines, if possible?

Copy link
Member Author

Choose a reason for hiding this comment

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

ファイル先頭のnoteに参考文献を書いています。基本的にはそこで定義されている補正係数です。

Copy link

@t-hosonuma t-hosonuma Oct 12, 2023

Choose a reason for hiding this comment

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

承知致しました,参考文献が書いてあればコメント不要という方針であれば問題ありません.
他に気になる点はありませんでしたので,Approveします.

Copy link
Member Author

Choose a reason for hiding this comment

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

わかりづらいと言う指摘だと思うので、コメントを追記しました。

Copy link
Member Author

Choose a reason for hiding this comment

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

すみません。pushしたつもりでマージしましたが、し忘れていたのでPRを作りました。クイックにレビューお願いします。

#515

@200km 200km merged commit d6e87ea into develop Oct 12, 2023
11 checks passed
@200km 200km deleted the feature/add-moon-rotation branch October 12, 2023 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
environment environment calculation minor update add functionality in a backwards compatible manner priority::medium priority medium
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Update CelestialRotation to be used for other planets
2 participants