-
Notifications
You must be signed in to change notification settings - Fork 9
Dependency injection #65
Conversation
…-3.5 into Dependency-Injection
Yaremadzulynsky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome. One note I have is that the SM constructor is starting to look a little cluttered with TM stuff. What we could do is make a function in the SM cpp file called something like setupTM() which will do the TM setup stuff, then call tm->init() after everything is set up.
d728939 to
58ebab6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 😃
larry-pan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
HardyYu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Description
What was completed, changed, or updated?
All dependency objects used by TelemetryManager (e.g. GroundStationCommunication) are now created in SystemManager before the TM object. Dependencies are passed directly to the TM constructor.
All state data pointers used by TM have been organized to a struct.
Why was this done (if applicable)?
TM can now be created from mock instances for testing.
The order of object construction can be changed, so we can wait until we have all dependencies ready before initializing a TM object.
TM objects can potentially be made outside of SystemManager
Testing
What unit tests were used to validate the code?
Documentation
Milestone number and name:
M3
Link to Asana task: https://app.asana.com/0/1203458353737758/1208312402823490/f
Reminders
Add reviewers to the PR
Mention the PR in the appropriate discord channel