Refactor sleep functions

[blakejameson]

Summary

I created the SleepHelper class, adding the safe_sleep, short_hibernate, and long_hibernate functions to the class. For Functions, I edited the constructor to be passed a parameter of a SleepHelper object.

How was this tested

• Added new unit tests
• Ran code on hardware (screenshots are helpful)
• Other (Please describe)

[Mikefly123]

@blakejameson thanks for the quick turn around on this work!

Before we rubber stamp this, could you run a quick test and show that directly calling all of the safe_sleep functions (ideally with various inputs) actually sleeps the satellite for the intended duration?

[blakejameson]

@Mikefly123 I appreciate you asking for some tests

In the image attached, I put an underscore under where I call the function and after the function call is over. It is still a lot to look through but the lines help somewhat.

One thing to note is that of course, as previous, duration logic works in increment of 15s. So far example, an input of 15 through 29 would result in an actual duration of 15. A duration input of 30 to 44 would result in an actual duration of 30, and so on. This is because of the 'while duration >= 15' logic combined with the 'duration-=15' that is featured in the while loop. The test furthest down was a duration input of 16 seconds but of course results in an actual duration of 15 seconds, as shown

[blakejameson]

@Mikefly123 And here is the code I put together in main.py to run the tests you see above

[blakejameson]

@Mikefly123 Snap, I just realized I misinterpreted your ask, and you probably wanted me to test short_hibernate and long_hibernate as well

[blakejameson]

@Mikefly123 It appears that short and long hibernate are erroring out since self.enable_rf is set to a boolean value. Saying 'bool.value = False' is causing an Exception, as shown in the output. I am going to stop working for the night and will get back at it tomorrow

[Mikefly123]

Hey @blakejameson, good catch on that hibernation bug!

I believe that is going on here is that the enable_rf object is usually supposed to be a digitalio.DigitalInOut which has an attribute value that you are able to call. Because the current code is casting it as a bool we have to just directly set it to true or false rather than calling enable_rf.value.

[blakejameson]

@Mikefly123 Here is testing the short and long hibernate functions. As shown, the short hibernate goes for 120 seconds, matching the safe_sleep(120) function call the function definition contains.

The long hibernate goes for 180 seconds, despite the safe_sleep(600) function call the function definition contains. This makes sense however, because with the safe_sleep function, the iterations logic only allows up to 12 iterations of 15 seconds. 12*15 = 180, so it makes sense that the safe_sleep will only go up to 3 minutes.

[blakejameson]

@Mikefly123 Also, given the potential exception seen earlier in this thread regarding the type of self.enable_rf, would a slight refactor accounting for the type be something you'd prefer? Below is an idea that would take into account if self.enable_rf is a DigitalInOut or a bool

[Mikefly123]

Hi Blake!

All here looks good, thanks for making the changes and running these tests. I like that recommendation of checking for how the enable_rf object is setup as a means of defining what kind of call we use to it. If you want to add that in and verify it is working I think we can rubber stamp this PR can get it merged.

I want to add just for context, in the near future we will want to refactor the hibernate functions to genuinely sleep the satellite for longer than 3 minutes at a time, but for Orpheus we opted not to do that to make operations safer (so we don't end up in a position where the satellite is unexpectedly uncommunicable). When we transition to V2 PROVES Kit, we'll want to bring back genuine hibernation functions that might allow the satellite to sleep for hours or even days at a time.

[blakejameson]

@Mikefly123 That context helps a ton! Thank you so much for explaining

[blakejameson]

@Mikefly123 Alright, so I went ahead and tested again with the newer way of checking the type of enable_rf. When 'legacy' is set to false in the config.json, the code works normal as shown below

[blakejameson]

@Mikefly123 However, I noticed that whether running the code on this branch here with 'legacy' set to true in the config.json or even on the main branch with 'legacy' set to true in the config.json, it errors out with the following message:

I guess that explains the Temporary Fix for RF_ENAB section in the code?

[Mikefly123]

@blakejameson good catch on that bug when legacy is set to True! Yeah, that error happens because the code is looking for a board.RF_ENAB pin in the firmware to initialize as the self.enable_rf digitalIO object. Ideally, we handle this error in such a way that still allows the code to run without having to force a reset.

I was actually just thinking, once the config.json write updates are made, this could be a way for us to autodetect what version of firmware is running and set the legacy variable to True automagically if this issue is detected. For now, I think it's fine for you to just push what you got and if we run with legacy set to False everything will be okay.

Could you submit an Issue Ticket that describes this legacy behavior as a bug so we don't forget about it?

[blakejameson]

@Mikefly123 Yes! I will create an issue ticket regarding legacy

And now after a recent push for my last commit, the code is ready for review

[Mikefly123]

LGTM! Thanks for these updates Blake

[nateinaction]

Good extraction from functions.py. @blakejameson since you have context right now could you add doc comments to the new SleepHelper class/methods?

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[blakejameson]

I went on to add documentation to the code and removed Config from the SleepHelper class/constructor, as it is currently not being used in the class.

As the constructor slightly changed and its call in main.py therefore changed, just to be sure the code works again, I went on to test the code on a v4b board, and it looks normal:

[nateinaction]

🚀

[blakejameson]

Hi Blake!

All here looks good, thanks for making the changes and running these tests. I like that recommendation of checking for how the enable_rf object is setup as a means of defining what kind of call we use to it. If you want to add that in and verify it is working I think we can rubber stamp this PR can get it merged.

I want to add just for context, in the near future we will want to refactor the hibernate functions to genuinely sleep the satellite for longer than 3 minutes at a time, but for Orpheus we opted not to do that to make operations safer (so we don't end up in a position where the satellite is unexpectedly uncommunicable). When we transition to V2 PROVES Kit, we'll want to bring back genuine hibernation functions that might allow the satellite to sleep for hours or even days at a time.

@Mikefly123 Had recently thought about safe sleep and am curious if now the work can be done to allow for sleeping longer than 3 minutes and also correcting the logic to allow for precise sleep duration (instead of the current 15-second increment way)

[Mikefly123]

Hey @blakejameson thanks for circling back on this!

If you would like to open a new ticket in the pysquared repo to improve the sleep_helper functions that would be great. A few things for you to consider:

• Being able to sleep for longer than 3 minutes would be great. Honestly, I think we should make this something that we can draw from config where the end user can set what they think is the longest allowable sleep time for their edition of flight software is.
• Davit has not been able to work on the RTC stuff in a while, but we should check in on whether the time alarm stuff that he was working on maybe works now and we can use that external RTC as a means of sleeping until precise times rather than just a relative number of seconds.

We can also move away from the 15 second increments but some gotchas to watch out for when doing that:

• V4c and lower flight controller boards do not have the ability to disable the hardware watchdog timer after it is enabled. This watchdog must be pet every 26s or else the hard reset it triggers will pull you out of sleep pretty aggressively
• V5a and above boards should have the ability to disable the external watchdog once you're in runtime, but this functionality still needs to be tested

[blakejameson]

@Mikefly123 Great, thanks so much!! I will go open a new ticket