Skip to content

Antenna Command and Firmware Integration#2

Draft
Gold872 wants to merge 21 commits intomainfrom
mars
Draft

Antenna Command and Firmware Integration#2
Gold872 wants to merge 21 commits intomainfrom
mars

Conversation

@Gold872
Copy link
Copy Markdown
Member

@Gold872 Gold872 commented Nov 18, 2024

No description provided.

Comment on lines +58 to +60
if (command.hasRoverCoordinatesOverrideOverride()) {
_handleGpsData(command.roverCoordinatesOverrideOverride, true);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And stop tracking here, so that the next GPS update doesn't immediately override our override

Comment on lines +68 to +71
if (_currentCommand.hasRoverCoordinatesOverrideOverride() &&
!isRoverOverride) {
return;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is tricky because having a nullable field is kind of "sticky":

  • the field starts off never having been sent: has() is false
  • the field is sent once, now has() is true
  • _if you use .merge(), has() will always stay true!

In this case you're not using merge, but it's not impossible to think we'd want to, and then this would start subtly causing bugs. What we can do instead:

  • have a GpsCoordinates field on this class for the roverPosition
  • when tracking, each _handleGpsData overrides that position
  • when a manual override is sent, override the position and take us out of tracking mode
  • now _handleGpsData won't even try to override the position until we start tracking again

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.

2 participants