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 distance sensor #15

Merged
merged 4 commits into from
Jul 25, 2022
Merged

Add distance sensor #15

merged 4 commits into from
Jul 25, 2022

Conversation

200km
Copy link
Member

@200km 200km commented Jul 19, 2022

概要

Add distance sensor

Issue

詳細

相対距離センサの雛形を追加。この方針で問題なければ相対位置センサや相対姿勢センサも加えていく。
このクラスにノイズ機能をつけておくか要相談。 -> ノイズ機能も付けることにした

検証結果

  • 標準偏差1m、定常バイアス10mに設定した場合のログ出力結果は下図の通り。想定通りの出力となっている。

image

影響範囲

NA

補足

NA

注意

NA

@200km 200km added the priority::high priorityg high label Jul 19, 2022
@200km 200km requested a review from suzuki-toshihir0 July 19, 2022 18:53
@200km 200km self-assigned this Jul 19, 2022
@200km
Copy link
Member Author

200km commented Jul 20, 2022

@suzuki-toshihir0 WIPつけていますが、方向性の確認だけしてもらえると嬉しいです。

@suzuki-toshihir0
Copy link
Member

方向性は問題ないと思います!
ノイズ機能をつけるかどうかについてですが,SensorBaseにノイズ機能がついているので,このクラスにもついている方が考え方が一貫していて良いだろうと思っています.いかがでしょうか.

@200km
Copy link
Member Author

200km commented Jul 20, 2022

はい。ノイズをつけるならSensorBaseを使うと思います。もともとのissueが「真値を取得できる」ということだったので、どうしようか迷っている感じです。

@suzuki-toshihir0
Copy link
Member

今考えると、真値にこだわる必要は無いように思いますね。。。ノイズ機能をこのでつけない理由は無いのかなと思っています。

@200km
Copy link
Member Author

200km commented Jul 20, 2022

次のどっちがほしいかですよね。

  • iniファイルなども考える必要なく、何も考えずに搭載すれば真値が得られるもの
  • iniファイルを適切に設定すれば、ノイズありも真値直接もどちらも得られるもの

S2Eに慣れている僕たちは後者で良いと思う気がするのですが、それすら面倒だと感じる人がいるから前者がいるということなんですかね?

@suzuki-toshihir0
Copy link
Member

なるほどです。
coreの考え方は基本的に後者だと思うので、s2e_ffで急に前者の考え方を取るのも変な気がしています。
あとは、デフォルトでノイズ機能を切っておけばよいだけにも思います。

@suzuki-toshihir0
Copy link
Member

あとは、s2e_ffは複数機のシミュレーションをやるという意味で、ある種発展的な使い方をしていると捉えています。s2e_ffのユーザーは、s2e_coreなどを使って単数のシミュレーションをして、多少使い方を学んだ人を想定してもよいだろうとも考えています。

@200km
Copy link
Member Author

200km commented Jul 21, 2022

わかりました。では、ノイズもつけておきます。
取り付け位置や座標系は無しで良いですかね?

@suzuki-toshihir0
Copy link
Member

ありがとうございます。
取り付け位置、向きは無くて良いかと思います。

@200km 200km changed the title WIP: Add distance sensor Add distance sensor Jul 21, 2022
@200km
Copy link
Member Author

200km commented Jul 21, 2022

@suzuki-toshihir0 ノイズ機能つけたのでレビューお願いします。

Copy link
Member

@suzuki-toshihir0 suzuki-toshihir0 left a comment

Choose a reason for hiding this comment

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

確認しました.(ReadSensorBaseInformationの置き場所だけ決められればapproveできそうです)

s2e-ff/data/ini/Components/RelativeDistanceSensor.ini Outdated Show resolved Hide resolved
s2e-ff/src/Simulation/Spacecraft/FfComponents.hpp Outdated Show resolved Hide resolved
@200km
Copy link
Member Author

200km commented Jul 22, 2022

I fixed the codes reflecting reviews.

Copy link
Member

@suzuki-toshihir0 suzuki-toshihir0 left a comment

Choose a reason for hiding this comment

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

修正点すべて確認しました!approveします.

@200km 200km merged commit 3133c86 into develop Jul 25, 2022
@200km 200km deleted the feature/add-distance-sensor branch July 30, 2022 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority::high priorityg high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants