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

Implement Elevation limits #107

Open
kmharrington opened this issue Sep 28, 2024 · 4 comments
Open

Implement Elevation limits #107

kmharrington opened this issue Sep 28, 2024 · 4 comments
Labels
good first issue Good for newcomers

Comments

@kmharrington
Copy link
Member

The SATs have minimum elevation limits. I just noticed those aren't in the configurations / never checked here. So far hasn't been an issue because we just know them (48 deg for satp1 and satp3, satp2 is set there right now but might get lowered). But we should have the software check for these.

@guanyilun
Copy link
Collaborator

is this as simple as adding assert el > 48 or similar?

@kmharrington
Copy link
Member Author

It does need to be configurable at some level, since I think SATp2's will end up being different. I guess the question is if it's better for it to be a "rule" or something that causes and error.

@guanyilun
Copy link
Collaborator

guanyilun commented Sep 30, 2024

I think it's good to error out so whoever requested that elevation will get the immediate feedback that it's not allowed. As a rule, the requested blocks may be transformed away and the feedback may be a bit hidden in the logs.

@kmharrington
Copy link
Member Author

yeah sounds good. We'll want to check all of the blocks not just the calibration target ones. Would also do el >= el_min instead of just equals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants