Skip to content
This repository was archived by the owner on Jun 13, 2023. It is now read-only.

Conversation

@tf-mac
Copy link

@tf-mac tf-mac commented Mar 4, 2023

Summary

This PR addresses Jira ticket CISLUNAR-###

  • All imports were redone to directly reference "fsw"
  • Main was slightly modified to use the "../" path in order to get this to work perfectly - There is definitively a better way of doing this
  • Errors arise when running in the simulator, however this has to do with how arguments are passed NOT regarding imports, therefore it is an issue for future work on integration

Testing

  • Run in both simulator and independent modes. Both did not work, however both crashed for seemingly unrelated issues:
    • The simulator crashed because of how data was passed to the flight mode, and in fact seems to come from some sort of syntax error
    • The independent mode crashed because it was not plugged into anything, giving errors about setting up the Raspberry PI

Notes

  • This is a first step in completing the integration of the simulator and the flight software, by allowing the flight software to be used as a submodule. This is important as if it is used as a package it could be very easy for a desync to occur since the simulator has to be manually updated. In this mode, git pull should pull both the simulator and the flight software
  • Future work has to be done to investigate why exactly the simulator crashes the flight software

Comments

  • Add an x between the brackets if you commented your code well!
    ^ made no changes other than renaming of imports, therefore N/A

- Almost all files will show an edit, as basically every file was changed
- VSCode decided to reformat in potentially damaging ways (redoing order of imports) so there may need to be a review on that change
- Imports in general need a redo. There was a mix of relative and absolute, and while its somewhat fixed now the hack in main needs to be replaced
Copy link
Contributor

@andrew032011 andrew032011 left a comment

Choose a reason for hiding this comment

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

This looks great! Thank you for completing this tedious task.

Something to think about...maybe for another ticket, @cameron-goddard? Part of the fixing of this repo will involve having a linter/standard for how we want to display import statements (i.e. alphabetical order, grouping the same modules together, etc.). That would make these refactoring changes a little easier in the future.

@tf-mac
Copy link
Author

tf-mac commented Mar 11, 2023

Quick review seems to confirm that my over-aggressive formatter made no damaging changes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants