[TASK] Extract Safe_Init to its Own Class

[Mikefly123]

Proposed Task

As part of our continuing efforts to simplify the code and make it easier for people to work with, we may want to consider abstracting the initialization of hardware into its own class called safe_hardware_init or something along those lines rather than having it occupy ~130 lines in the satellite class right now. The way I see extracting all of those functions and then passing them back in as a generic safe_hardware_init(args) call of some sort would make the code more modular and fit the Dependency Injection pattern we are trying to inhabit.

Possible Acceptance Criteria

• safe_init functions moved into a dedicated class that is imported into pysquared.py
• When a hardware object needs to be initialized we call to safe_hardware_init to create and return that object
• A parser for whether we are trying to initialize specific hardware objects or generic ones
• Some kind of mechanism for deinit and reinit of the hardware as needed

Technical Notes

Let me know what y'all think about this task, I kinda teeter between whether or not this would be a useful abstraction right now or just complicating things further.

[Mikefly123]

Paging @dbgen1 for your thoughts on this one because you originally created the safe_init stuff

[dbgen1]

I think this is a good idea, and I agree with the idea that deinit and reinit should be something that is useful. But then, the scope of the class goes beyond just being an "initializer". @Mikefly123 what do you think of a similar idea implementing all the bullet points called "HardwareManager"? I'd also like to add that the class could store the hardware status dictionary that is currently in pysquared, as it makes more sense for it to go along with the class that controls initialization and deinitialization of hardware

[Mikefly123]

Hey @dbgen1 I think creating a generic hardware manager class could be a pretty good thing for us to do. It would also possibly clean up all of those individual driver imports that are currently piled up inpysquared.py.

Based on the conversation we had with Nate yesterday, I like the idea of trying to follow somewhat in the footsteps of F Prime and get towards a more component based architecture. If we can try and make pysquared.py just the central file that stitches together all of the component interfaces, but doesn't actually handle any of them itself, I think that would be a solid way of moving forward!

[dbgen1]

Here's another crazy idea: we can also delegate all hardware calls to such a class. This is bit of a radical step, but here's the general idea. Right now, whenever we want to call hardware in pysquared, we do the following:

1. Check if the device is initialized, print a warning if not.
2. If the device is initialized, wrap the hardware call in a try catch with the same logger statement every time.

Doing these things manually every time is time consuming and easy to mess up, so it would be ideal if our hardware manager could handle these things for us. How can we do this? I propose the following:

1. Store devices in HardwareManager instead of pysquared. So for example, when setting the rtc time in pysquared, instead of self.rtc.set_time(), we would have self.hardware.rtc.set_time(). Here, rtc is a property of the HardwareManager that automatically checks if the rtc is initialized when enabled.
2. (This is the crazy part) When initializing a hardware component, we use a metaclass to automatically apply a wrapper to every function in that hardware's driver. This wrapper essentially forces every hardware call to go through a try catch which automatically applies the error logging with the logger class if something goes wrong.

Now, our pysquared code goes from:

if self.hardware["RTC"]:
    try:
        self.rtc.set_time(whatever)
    except Exception as e:
        self.logger.error(something or other)
else:
    self.logger.warning("oops that device isn't initialized!!)

to just:

self.hardware.rtc.set_time()

Pros

• slims down pysquared code and makes interacting with hardware a lot easier
• reduces risk of error (i.e forgetting to add these checks) when writing code that calls hardware

Cons

• metaclasses are really weird.
• makes adding NEW hardware difficult (proper documentation and instructions can maybe address this)
• standardizing error handling for all hardware calls may cause issues when/if custom handling is needed.

@Mikefly123 @nateinaction let me know what you think of this idea. I would appreciate candid feedback.

[nateinaction]

I plan to template out some ideas on this topic tomorrow. I'll share more with some kind of POC/code sample.

[Mikefly123]

I like this idea!

I will admit as someone not trained as a software engineer I am still catching up on these concepts. From what I understand, the main debate here would be between using a meta class (which is one of the ways to enforce an interface in Python?) that would be an abstract base for all of our hardware manager classes or individually implementing error handling in each hardware manager class using something like a __getattr__() block embedded in each class?

I am a fan of the idea that we can try to standardize our error handling approach for all of the devices. I think there is some architectural thinking to be done here about what kind of errors are "soft" and can accumulate with just a log message and what kind of errors are "hard" and require specific action to be taken by the code to try and correct it.

Another factor we'll want to consider is that if certain pieces of hardware are malfunctioning (or simply not connected in the case of a flight controller board existing on its own) we want some kind of graceful way for the code to understand that state and block certain functions from executing rather than just fumbling into them and throwing errors because the hardware isn't there.

Looking forward to seeing what Nate has in mind! I will also try and take some time to document specific examples of how we have thought about error handling in the past.

[dbgen1]

I appreciate both of you taking the time to share and discuss these things. Looking forward to what Nate has to say!

[nateinaction]

Hey team, I spent some time on this on Sunday and decided to use the factory pattern to extract the radio instantiation from the Satellite class. Sorry for the dirty PR (lots of unrelated changes) but the primary files to look at to see the way it was extracted are main.py and lib/pysquared/radio.py. #136 #140

I suspect that we are going to have different behaviors that we will want to implement when a specific piece of hardware fails. I suspect that if the radio fails we want to retry a few times and then reboot. Are there other kinds of hardware where we might need to do the same thing? The SPI interface required by the radio comes to mind.

I'm still sitting on the on the opposite end of the field from the goal but where I'm sitting right now, I'd kind of like to try extracting every hardware component to its own class because those classes will be smaller and simpler to reason about. We'll be able to more easily wrap those classes in our desired behavior and future hardware using pysquared will be easier to implement.

Any chance we could try to classify the behavior we want to see when each piece of hardware is instantiated? That might help inform some of our decisions.

[nateinaction]

We might want find a few shared decorators helpful with all hardware classes. A @retry with variable for number of tries and a backoff. A @reboot_on_failure decorator maybe?

[nateinaction]

I've cleaned up my previous PR and given a good swing at the radio and hardware pins in #140. Let me know what you guys think and if you have thoughts on this change.

[dbgen1]

The factory pattern is smart and I like the idea of the shared decorators. I see you implemented retry as well as a hardware initialization error! This looks like a really good PR and a template on how to do the other hardware devices!

[Mikefly123]

I'm with Davit, the rfm9x_factory.py looks really nice and clean! Do you foresee us extracting all of the hardware initializations into their own unique classes using the factory pattern or would be make them more generic with like an i2c_sensor_factor and an spi_sensor_factory sorta thing? Mainly because most of the other devices don't need as many (or often any) parameters passed to them on init.

[dbgen1]

We talked about this very briefly, and we agreed that it depends on the use cases for each of the hardware sensors. In particular, we might get away with a single factory for the i2c devices, as a lot of them are very similar. I'll try to write something up and submit it as a draft PR this week!

[Mikefly123]

Awesome! Looking forward to seeing it!

[Mikefly123]

Just to close the loop on this thread we are going to be moving forward with the factory pattern for hardware inits laid out in #140. We'll go ahead and close this issues once #140 is merged!