Skip to content

Conversation

@hender14
Copy link
Contributor

@hender14 hender14 commented Dec 23, 2025

概要

FSW 側で ti の値がオーバーフローしない様に、new_epoch の値を小さくする

Issue

検証結果

  • 0.7~1.3 の間でランダムに設定される CYCLES_PER_SEC の補正値が最大でもオーバーフローしない事を確認

補足

別途、c2a-pytest-gaia 側の epoch 設定を変える API が無いという問題はあり、此方は arkedge/c2a-pytest-gaia#8 にて対応していく

Copilot AI review requested due to automatic review settings December 23, 2025 11:22
@hender14 hender14 self-assigned this Dec 23, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the calculation method for new_epoch in the time manager test to improve test determinism. The change replaces a non-deterministic calculation based on the current time with a fixed offset from the default epoch.

  • Changed new_epoch calculation from time.time() - 86400 * 30 to TMGR_DEFAULT_UNIXTIME_EPOCH_FOR_UTL + 86400 * 30
  • Updated the comment to accurately reflect the new calculation method

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@hender14
Copy link
Contributor Author

@meltingrabbit cc @sksat
test_time_manager.py について、new_epoch で設定した値次第で fsw 側の unixtime → ti 変換 & c2a_round 処理した際に値がオーバーフローしている問題が確認されたため、new_epoch の値を小さくしてオーバーフローを回避できるように修正を行いました。
確認お願いします。

@meltingrabbit
Copy link
Member

@hender14
オーバーフロー,なるほど,ありがとうございます.

これだと,デフォルトエポックが,
30日前~テスト実行時
の間だと,新しいエポックが未来になってバグる気がします.

ただ,エポックがそんなに最近なこともない気がするので,大丈夫な気もします.

という悩ましい感じですね.

@hender14
Copy link
Contributor Author

@meltingrabbit
了解です。epoch 未来の時間になっていると、unixtime の相対時間を算出する際にアンダーフローする可能性があるという事ですかね。

根本の対策にはなっていませんが、コメントで、epoch 時間が現在時刻-30day にならない様記載すると共に、実際に epoch が未来の時刻で設定された場合は assert でエラーとする事で対応できればと思いますが、如何でしょうか。

# NOTE: テスト内でこの値に +30日 などを加算して検証するケースがあるため、
#       現在時刻に対して十分過去(少なくとも30日以上前)の日時である必要がある。
TMGR_DEFAULT_UNIXTIME_EPOCH_FOR_UTL = 1577836800.0
# new_epoch が未来になっていないかチェックし、失敗したらメッセージを出す
assert new_epoch < time.time(), "設定された Epoch が未来時刻になっている"

@meltingrabbit
Copy link
Member

@hender14 よさそうです!

@hender14
Copy link
Contributor Author

@meltingrabbit
d7fd108 にてコード反映しました。
確認お願いします。

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.

良さそうです.ありがとうございます!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants