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

fix: unit conversion bug in to timeval (backport #24) #2

Merged
merged 1 commit into from Jun 13, 2023

Conversation

v-nojiri7841-esol
Copy link

Description

select関数のタイムアウト時間について、単位変換の誤りを修正する。

fork元の以下のPRの取り込みです。
autowarefoundation#24

発生していた問題事象

sockec_can_receiverについて、interval_secを0.02(単位:sec)で呼び出すと、CPU使用率が100%になり、1秒間隔でWarningが表示される。

# 起動コマンドの一例
ros2 launch ros2_socketcan socket_can_bridge.launch.xml interface:='canIMU' receiver_interval_sec:='0.02'
# or
ros2 launch ros2_socketcan socket_can_receiver.launch.py interface:='canIMU' interval_sec:='0.02'
# Warningメッセージ
[socket_can_receiver_node_exe-1] [WARN] [1686291362.684238888] [socket_can_receiver]: Error receiving CAN message: canIMU - Resource temporarily unavailable

発生メカニズム

select関数はソケットの準備待ち(*1)をするが、この時のタイムアウト時間はROS2のパラメータで渡される値を使う。

パラメータはdouble型(単位:sec)に対し、select関数はtimeval型でタイムアウトを設定する事から単位変換が発生する。
単位変換の途中にnanosecからmicrosecへの変換が含まれるが、1000で除算するところを乗算しており、意図しない値が代入されていた。

結果、意図しないタイムアウト時間でselect関数が呼び出され問題が生じていた。

変換の流れは以下の通り。timeval型は整数部と小数点以下を別々の構造体メンバーで保持する。
小数点以下の部分は内部で次のように変換されている。

正:0.01(s) -> 10_000_000(ns) -> 10_000(ms)
誤:0.01(s) -> 10_000_000(ns) -> 10_000_000_000(ms)

小数部分を格納する領域に、小数の範囲を超える値が格納され意図しない動作になっていると考えられる。

*1 sockec_can_receiverはデータ読み込み可能になったか、socket_can_senderはデータ書き込み可能になったかを指す。

Related links

Tests performed

  • CAN-BUSから与えるCANフレームの量を調整し、sockec_can_receiverのinterval_secが期待通り動作していることを確認。
    • 以下の事前準備を実施した後、パターン1とパターン2を実施して確認。
  • sockec_can_senderのtimeout_secによりノードが意図しないWarning/Errorを検出しないことを確認。
    • Fork元のPRではsockec_can_senderの動作は言及されていないが、修正箇所の関数を使っているので確認必要と判断している。
    • 以下の事前準備を実施した後、パターン3を実施して確認。
      • 次の理由により、select関数で待ちを起こすのは困難であることから、通信を発生させて意図しないWarning/Errorを検出しないことで最低限のデグレを確認する。
        • 1つのCANデバイスを1ノードが占有することから、自分自身のsendの呼び出しが終わったら次のselect関数を呼ぶこととなる。この動きにより、書き込み待ちになりそうなタイミングでselect関数を呼ぶことが困難である。
事前準備

sudo apt install can-utils
sudo modprobe vcan
sudo ip link add dev vcan0 type vcan
sudo ip link set up vcan0

パターン1:interval_sec > 受信間隔

# Terminal-1
cangen vcan0 -g 19
# Terminal-2
top
# Terminal-3
ros2 launch ros2_socketcan socket_can_receiver.launch.py interface:='vcan0' interval_sec:='0.02'

上記を実施した状態で以下を確認する。

  • topに表示されるsocket_can_receiverのCPU使用率が100%付近でないこと。
    • 参考:12th Gen Intel(R) Core(TM) i7-12700Fでは数%にも満たない使用率となる。
  • Warning[socket_can_receiver_node_exe-1] [WARN] [実行時の時間] [socket_can_receiver]: Error receiving CAN message: vcan0 - CAN Receive Timeoutが出力されないこと。

パターン2:interval_sec < 受信間隔

# Terminal-1
cangen vcan0 -g 21
# Terminal-2
top
# Terminal-3
ros2 launch ros2_socketcan socket_can_receiver.launch.py interface:='vcan0' interval_sec:='0.02'

上記を実施した状態で以下を確認する。

  • topに表示されるsocket_can_receiverのCPU使用率が100%付近でないこと。
    • 参考:12th Gen Intel(R) Core(TM) i7-12700Fでは数%にも満たない使用率となる。
  • Warning[socket_can_receiver_node_exe-1] [WARN] [実行時の時間] [socket_can_receiver]: Error receiving CAN message: vcan0 - CAN Receive Timeoutが出力されること。

パターン3:timeout_sec > 送信間隔

# Terminal-1
 ros2 topic pub -r 51 /to_can_bus can_msgs/msg/Frame '{id: 512, dlc: 8}'
# Terminal-2
top
# Terminal-3
ros2 launch ros2_socketcan socket_can_sender.launch.py interface:='vcan0' timeout_sec:='0.02'

上記を実施した状態で以下を確認する。

  • topに表示されるsocket_can_senderのCPU使用率が100%付近でないこと。
    • 参考:12th Gen Intel(R) Core(TM) i7-12700Fでは数%にも満たない使用率となる。
  • Warning/Errorが出力されないこと。

Notes for reviewers

None.

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

  • The PR follows the pull request guidelines.
  • The PR has been properly tested.
  • The PR has been reviewed by the code owners.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

  • There are no open discussions or they are tracked via tickets.
  • The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

Signed-off-by: v-nojiri7841-esol <v-nojiri7841@esol.co.jp>
Copy link

@m-iwashita-esol m-iwashita-esol left a comment

Choose a reason for hiding this comment

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

autowarefoundation#24
が正しく取り込まれている事を確認しました

@v-nojiri7841-esol v-nojiri7841-esol changed the title Fix: unit conversion bug in to timeval fix: unit conversion bug in to timeval Jun 13, 2023
@h-ohta
Copy link

h-ohta commented Jun 13, 2023

@v-nojiri7841-esol 実質的にbackportのPRなので、タイトルに (backport autowarefoundation#24)と最後に加えてもらえますか?

@v-nojiri7841-esol v-nojiri7841-esol changed the title fix: unit conversion bug in to timeval fix: unit conversion bug in to timeval (backport autowarefoundation#24) Jun 13, 2023
@v-nojiri7841-esol
Copy link
Author

@v-nojiri7841-esol 実質的にbackportのPRなので、タイトルに (backport autowarefoundation#24)と最後に加えてもらえますか?

@h-ohta タイトルにバックポートの情報を加筆いたしました。

@h-ohta
Copy link

h-ohta commented Jun 13, 2023

あ、ごめんなさい#24だけで大丈夫です
つまり (backport #24)

@h-ohta h-ohta changed the title fix: unit conversion bug in to timeval (backport autowarefoundation#24) fix: unit conversion bug in to timeval (backport #24) Jun 13, 2023
@h-ohta
Copy link

h-ohta commented Jun 13, 2023

私の方で直しておきました

@h-ohta
Copy link

h-ohta commented Jun 13, 2023

squash mergeする際にPRタイトルが適用されないようなので、気をつけてください

@h-ohta h-ohta merged commit 994e631 into beta/x1.eve/1.0.1 Jun 13, 2023
@h-ohta h-ohta deleted the fix/unit_conversion_bug_in_to_timeval branch June 13, 2023 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants