Comms expansion #209
Comms expansion #209
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
…nto comms_expansion
|
@gomezzz. I cannot ask for your review, but it would be good if you could also take a look at this PR. |
There was a problem hiding this comment.
A few comments :) Overall quite nice!
Two main concerns for me atm:
- Please add a minimal example to the README.MD on how to use this
- See the longer comment in one of the models, you are using a lot of different classes that are very similar. I would suggest to simplify that
examples/Communication_example/communication_simple_satellite.ipynb
Outdated
Show resolved
Hide resolved
examples/Communication_example/communication_simple_ground.ipynb
Outdated
Show resolved
Hide resolved
| """Helper function to track comms status""" | ||
| conv_values = [] | ||
| for val in values: | ||
| status = ["No signal", "Ground only", "CommSat only", "Ground + Sat"] |
There was a problem hiding this comment.
status shall be implemented as enum, not as string.
| ), "An optical receiver is required for this optical link." | ||
|
|
||
| logger.debug("Initializing optical link model.") | ||
| self.required_BER = 10e-3 |
There was a problem hiding this comment.
Why 10-3? I would have this value as user-defined value. In addition, this is typically linked with the maximum data-rate that you can have.
There was a problem hiding this comment.
For optical this the BER for which the equation to determine bitrate is valid. It was quite difficult to find an equation where the bitrate could be calculated from. The one I could find so far is only valid for BER 10E-3 and a wavelength of 1550 nm.
| self.required_BER = 10e-3 | ||
|
|
||
| self.modulation_scheme = "OOK" | ||
| self.required_s_n_margin = 3 # in dB |
There was a problem hiding this comment.
Is S_N_margin linked to the desired BER? If yes, we should avoid the definition of two related constants.
|
|
||
| self.total_channel_loss = self.get_path_loss(slant_range) | ||
|
|
||
| self.signal_at_receiver = self.transmitter.EIRP - self.total_channel_loss |
There was a problem hiding this comment.
Are those values on a logarithmic scale? If yes, please, specify it in the names (e.g., EIRP_dB or something like that).
|
|
||
| self.total_channel_loss = self.get_path_loss(slant_range) | ||
|
|
||
| self.signal_at_receiver = self.transmitter.EIRP - self.total_channel_loss |
There was a problem hiding this comment.
If these are in logarithmic scales, that means the loss of your signal will reduce your power to 0 dB. The effect of that depends on the noise power. If the noise power is higher than 0 dB, then you will have a terrible Bit Error Rate.
| self.required_BER = 10e-5 | ||
| self.frequency = frequency # in Hz | ||
| self.modulation_scheme = "BPSK" | ||
| self.zenith_atmospheric_attenuation = 0.5 # in dB | ||
| self.required_s_n_ratio = 9.6 # in dB | ||
| self.required_s_n_margin = 3 # in dB |
There was a problem hiding this comment.
Again, I would let the user specify margins and the other parameters.
|
Opened issue #210 for some possible improvements in the future. |
Description
Summary of changes
Added link budget modelling for communication, optical and radio.
Added Python notebooks to test/show satellite to ground (radio) and intersatellite (optical)
Still working on a bug where the notebooks throw a circular reference error on imports.
Resolved Issues
How Has This Been Tested?
Calculations have been tested by comparing hand calculations with papers and SMAD.
Software models have been tested by creating unit tests.