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

Ground station implementation #68

Merged
merged 10 commits into from
Dec 20, 2022
Merged

Ground station implementation #68

merged 10 commits into from
Dec 20, 2022

Conversation

gomezzz
Copy link
Collaborator

@gomezzz gomezzz commented Dec 1, 2022

Description

Summary of changes

  • Implemented ground station actor with vision cones etc.
  • Some bugfixes along the way from bugs introduced in Set position only once #45
  • Added some missing docstrings
  • Added skyfield

Resolved Issues

How Has This Been Tested?

  • pytest
  • test driven development

Related Pull Requests

N/A

@gomezzz gomezzz added tests Anything related to the tests feature implementing a new feature user-facing Anything that users can interact with physical-model All things involving some physical models labels Dec 1, 2022
Copy link
Collaborator

@GabrieleMeoni GabrieleMeoni left a comment

Choose a reason for hiding this comment

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

I still need to test it, but I would like to understand it better first.

paseos/actors/actor_builder.py Outdated Show resolved Hide resolved
paseos/actors/actor_builder.py Outdated Show resolved Hide resolved
paseos/actors/actor_builder.py Show resolved Hide resolved
paseos/actors/base_actor.py Outdated Show resolved Hide resolved
paseos/actors/ground_station_actor.py Show resolved Hide resolved
paseos/communication/is_in_line_of_sight.py Show resolved Hide resolved
paseos/communication/is_in_line_of_sight.py Show resolved Hide resolved
@gomezzz gomezzz mentioned this pull request Dec 13, 2022
1 task
@gomezzz
Copy link
Collaborator Author

gomezzz commented Dec 14, 2022

@GabrieleMeoni I am waiting on a review of this if you find a second 🙏 (starting to accumulate a lot of PRs :D )

Copy link
Collaborator

@GabrieleMeoni GabrieleMeoni left a comment

Choose a reason for hiding this comment

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

Everything works fine for me, but I would add:

-) Comments to explain the rationale of tests (at a first glance, satellites with 0 velocity are weird. Also, it is not immediate to understand why satellites becomes visible over time).

-) Test to check GS visibility with non-zero satellite velocity. Maybe S2 (tle are notes and GS positions should be retrievable) and check how long the satellite is visible. This should give us a reasonable Ground-truth.

environment.yml Show resolved Hide resolved
@gomezzz
Copy link
Collaborator Author

gomezzz commented Dec 19, 2022

@GabrieleMeoni

-) Test to check GS visibility with non-zero satellite velocity. Maybe S2 (tle are notes and GS positions should be retrievable) and check how long the satellite is visible. This should give us a reasonable Ground-truth.

Do you have this data? I don't have it, especially the altitude angle at which the station are able to talk to S2 and window length I can't find online and I don't wanna spend hours searching.

@gomezzz
Copy link
Collaborator Author

gomezzz commented Dec 20, 2022

Added test for groundstation window length

@gomezzz gomezzz requested review from GabrieleMeoni and removed request for GabrieleMeoni December 20, 2022 16:09
- More logging
- New test for reality check
@gomezzz gomezzz merged commit 597a08d into main Dec 20, 2022
@gomezzz gomezzz deleted the ground-station-implementation branch December 20, 2022 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature implementing a new feature physical-model All things involving some physical models tests Anything related to the tests user-facing Anything that users can interact with
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ground stations
2 participants