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 antenna radiation pattern read from CSV file #284

Merged
merged 24 commits into from
Dec 30, 2022

Conversation

200km
Copy link
Member

@200km 200km commented Dec 28, 2022

Overview

Add antenna radiation pattern read from CSV file

Issue

Details

Add antenna radiation pattern feature read from CSV file.

Validation results

NA

Scope of influence

Only for communication and ground station analysis.

Supplement

NA

Note

NA

@200km 200km added priority::high priorityg high component component emulation minor update add functionality in a backwards compatible manner labels Dec 28, 2022
@200km 200km self-assigned this Dec 28, 2022
@200km
Copy link
Member Author

200km commented Dec 28, 2022

これが原因か、、、

Windows環境においては、<windows.h>をインクルードするとmaxという名前の関数マクロが定義され、std::max()と衝突してしまうという問題がある。

double gain_dB = 0.0;
switch (ant_params.antenna_gain_model) {
case AntennaGainModel::ISOTROPIC:
gain_dB = ant_params.gain_dBi_;
Copy link
Member

Choose a reason for hiding this comment

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

AntennaGainModel::ISOTROPICとは何ですか?

Copy link
Member Author

Choose a reason for hiding this comment

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

enum定義で説明コメントを付けていたと思いますが、等方性アンテナのことです。アンテナパターンCSVファイルが無いときは、理想的な等方性アンテナとして扱うしか無いかなと言う感じです。アンテナパターン実装を行う前は、等方性アンテナと同じ事をやっていたと認識しています。

Copy link
Member

Choose a reason for hiding this comment

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

なるほどです.ありがとうございます.

if (is_tx) {
return tx_gain_;
double Antenna::CalcTxEIRP(const double theta_rad, const double phi_rad) const {
return tx_eirp_dBW_ - tx_params_.gain_dBi_ + CalcAntennaGain(tx_params_, theta_rad, phi_rad);
Copy link
Member

Choose a reason for hiding this comment

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

tx_params_.gain_dBi_は足すのではなく引くんですか?

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.

数式としては、もともとから

  • $P_{tx}$: RF output power for transmission [W]
  • $G_{tx}$: Transmit maximum gain [dBi]
  • $GA_{tx}$: Antenna gain [dB]
  • $L_{f_{tx}}$: Feeder loss [dB]
  • $L_{p_{tx}}$: Pointing loss [dB]
  • $EIRP_{tx}$: Transmit EIRP [dBW]
$$EIRP_{tx} = 10 \log_{10}{P_{tx}} + G_{tx} + L_{ftx} + L_{ptx} - G_{tx} + GA_{tx}$$

となっていて、アンテナパターンが入っていなかったときは

$$GA_{tx}=G_{tx}$$

としていたので、結局は

$$EIRP_{tx} = 10 \log_{10}{P_{tx}} + G_{tx} + L_{ftx} + L_{ptx}$$

となっていたようです。これがそもそも違うということなら、ただしい式を教えてもらえると嬉しいです。

Copy link
Member

@TomokiMochizuki TomokiMochizuki Dec 29, 2022

Choose a reason for hiding this comment

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

$G_{t_x}$を足してから同じ $G_{t_x}$ を引いているのがなんか違和感ありますね.
少し調べてみます.

Copy link
Member Author

Choose a reason for hiding this comment

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

はい。なんか無駄なことをやっているので変な気はしています。
等方性アンテナとそうでないアンテナとで無理やり同じ式を使おうとしているから7日も知れません。アンテナモデルが変わると数式も変わるというように実装したほうがスッキリする気もするので、そうしても良いかなと思います。

ただ、僕はあまり通信系に詳しくなく、単位系なども含めて正しさチェックを完全にできないので、前の実装を踏襲しているという感じです。

Copy link
Member

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.

単位系についてはどうでしょうか?アンテナゲインをdBとするかdBiとするかどちらにするべきでしょう。
元々は、

//送信ゲイン[dBi]
tx_gain = 8.0

みたいなのが、上の式の

  • $G_{tx}$: Transmit maximum gain [dBi]
    として使われていました。CSVで読み込むパターンは[dB]ですか?この辺り単位を統一したほうが良いですか?

Copy link
Member

Choose a reason for hiding this comment

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

すみません、見落としていました。
dBiは完全無指向性のアンテナを基準とした単位ですが、今回は双方ともこれに当たるので、dBiでいい気がします。

基準を示さずにdBとしてもいいかもしれないですが、dBiのほうが丁寧な印象を受けます。

Copy link
Member Author

Choose a reason for hiding this comment

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

CSVの値もdBiということで、揃えようと思います。ありがとうございます。

Copy link
Member Author

Choose a reason for hiding this comment

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

こちら、単位の修正と数式の単純化を行いました。

Comment on lines 24 to 35
double theta_rad_clipped = theta_rad;
double phi_rad_clipped = phi_rad;
if (theta_rad_clipped < 0.0) theta_rad_clipped = 0.0;
if (theta_rad_clipped > theta_max_rad_) theta_rad_clipped = theta_max_rad_;
if (phi_rad_clipped < 0.0) phi_rad_clipped = 0.0;
if (phi_rad_clipped > phi_max_rad_) phi_rad_clipped = phi_max_rad_;

// Calc index
size_t theta_idx = (size_t)(length_theta_ * theta_rad_clipped / theta_max_rad_);
if (theta_idx > length_theta_) theta_idx = length_theta_;
size_t phi_idx = (size_t)(length_phi_ * phi_rad_clipped / phi_max_rad_);
if (phi_idx > length_phi_) phi_idx = length_phi_;
Copy link
Member

Choose a reason for hiding this comment

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

ここのロジックがあまり詳しくわかっていないのですが,theta_radとphi_radを度数表記したうえで,整数に丸め込んでいるんでしょうか?

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

Choose a reason for hiding this comment

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

なるほどです.ありがとうございます.

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.

回線計算内容は別でレビューされていると思うので,それ以外の部分で1点質問しています.

src/Component/CommGS/AntennaRadiationPattern.cpp Outdated Show resolved Hide resolved
@sksat
Copy link
Collaborator

sksat commented Dec 30, 2022

これが原因か、、、

Windows環境においては、<windows.h>をインクルードするとmaxという名前の関数マクロが定義され、std::max()と衝突してしまうという問題がある。

あーーー引き当てちゃいましたか.有名な問題なんですよねこれ.windows.h 本当に行儀が悪い......

@200km 200km requested a review from seki-hiro December 30, 2022 09:04
@200km
Copy link
Member Author

200km commented Dec 30, 2022

@TomokiMochizuki @seki-hiro 頂いたコメント受けて諸々修正し終わったので、再レビューお願いします。

@@ -1,94 +1,146 @@
[ANT1]
Copy link
Member

Choose a reason for hiding this comment

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

[Q] ANT.iniとANT_GS/SC.iniが存在している理由がわからなかったのですが,どのような使い分けがされているのでしょうか?

Copy link
Member Author

Choose a reason for hiding this comment

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

このPRと関係ない部分ですが、ただのサンプルなので使い分けなどはなく、こういう使い方ができますよというものだと思います。

Copy link
Member

Choose a reason for hiding this comment

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

分かりました,ありがとうございます.

//受信用かのフラグ
is_receiver = 0
// Flag for receiver or not
is_receiver = 1
Copy link
Member

Choose a reason for hiding this comment

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

[Q] is_transmitter = 1かつis_receiver = 1という場合はありえるのでしょうか?送信または受信のいずれかであるならば,フラグは1つの方が管理しやすそうとも思いました.

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

Choose a reason for hiding this comment

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

手元だと0から1に変わっているように見えるので質問しました.エラーが起きたりしていないのであれば問題ないと思います.

Copy link
Member Author

Choose a reason for hiding this comment

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

変えていないというのは、is_transmitteris_receiverが別れていて、フラグ1つにするという部分に対するものです。  
値は変えていて、送受兼用アンテナの場合でも問題なく計算できるかを確かめています。

Comment on lines +71 to +72
rx_theta_max_rad = 6.28
rx_phi_max_rad = 3.14
Copy link
Member

Choose a reason for hiding this comment

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

[IMO] 角度の定義はコメントで記載し,対応する最大値自体はLibraryのpi, tauを使った方が良いのではないかと思いましたが,いかがでしょうか?

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

Choose a reason for hiding this comment

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

ユーザー指定の部分ということで承知しました.

//受信用かのフラグ
is_receiver = 0
// Flag for receiver or not
is_receiver = 1
Copy link
Member

Choose a reason for hiding this comment

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

手元だと0から1に変わっているように見えるので質問しました.エラーが起きたりしていないのであれば問題ないと思います.

@@ -1,94 +1,146 @@
[ANT1]
Copy link
Member

Choose a reason for hiding this comment

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

分かりました,ありがとうございます.

Comment on lines +71 to +72
rx_theta_max_rad = 6.28
rx_phi_max_rad = 3.14
Copy link
Member

Choose a reason for hiding this comment

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

ユーザー指定の部分ということで承知しました.

@200km 200km merged commit 37d3192 into develop Dec 30, 2022
@200km 200km deleted the feature/add-antenna-pattern branch December 30, 2022 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component component emulation minor update add functionality in a backwards compatible manner priority::high priorityg high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants