Skip to content

[#265] Maximum range checking model#267

Merged
Mark2000 merged 1 commit into
developfrom
feature/265-range-checking
Jun 5, 2025
Merged

[#265] Maximum range checking model#267
Mark2000 merged 1 commit into
developfrom
feature/265-range-checking

Conversation

@Mark2000

Copy link
Copy Markdown
Contributor

Description

Closes #265

Adds a max range checking feature to force a deputy to stay within a radius of a chief.

Also refactors indexing satellites from the simulator.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How should this pull request be reviewed?

  • By commit
  • All changes at once

How Has This Been Tested?

Added integrated test.

Future Work

Potential to extend to multiple satellites, but probably not needed.

Checklist

  • I have performed a self-review of my code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation and release notes
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

Copilot AI review requested due to automatic review settings May 28, 2025 00:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements a maximum range checking feature between satellites with an accompanying integration test and updates to satellite command generation. Key changes include:

  • Adding the MaxRangeDynModel in the dynamics module to report range violations.
  • Refactoring the _satellite_command in the Satellite class to use Simulator.get_satellite for cleaner string evaluation.
  • Introducing integration tests for the max range checking logic.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/unittest/sats/test_satellite.py Updated tests to bind get_satellite using partial for eval calls.
tests/integration/sim/test_int_dynamics.py Added integration tests for the maximum range dynamics model.
src/bsk_rl/sim/simulator.py Added a get_satellite method to look up satellites by name.
src/bsk_rl/sim/dyn.py Added MaxRangeDynModel with event-based range checking.
src/bsk_rl/sats/satellite.py Modified _satellite_command to simplify command generation.
docs/source/release_notes.rst Updated release notes to document the new feature.

Comment thread src/bsk_rl/sim/dyn.py Outdated
Comment thread src/bsk_rl/sim/dyn.py
@Mark2000
Mark2000 force-pushed the feature/265-range-checking branch 2 times, most recently from f426897 to 388934b Compare May 28, 2025 00:12
@Yume27 Yume27 assigned Yume27 and unassigned Yume27 May 30, 2025
@Yume27
Yume27 self-requested a review May 30, 2025 21:59
Comment thread src/bsk_rl/sim/dyn.py

@Yume27 Yume27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Like the get_satellite function, and passed all pytest locally.

@Mark2000
Mark2000 force-pushed the feature/265-range-checking branch from 388934b to e70f5cf Compare June 5, 2025 21:10
@Mark2000
Mark2000 merged commit f0b9c2a into develop Jun 5, 2025
@Mark2000
Mark2000 deleted the feature/265-range-checking branch June 5, 2025 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Range checking dynamics model

3 participants