USSTRocketry / MiniRockets

Making rockets that hopefully go UP!
MIT License
1 stars 11 forks source link

Fix blocking code in main program #57

Closed dhaussecker closed 1 year ago

dhaussecker commented 1 year ago
shitcode

Please fix the following lines of code to have a non-blocking implementation

frroossst commented 1 year ago

I don't get paid enough to deal with this shit

frroossst commented 1 year ago

How do we ensure non blocking implementation of the above, does not break soft_recovery_mode()

https://github.com/USSTR-Avionics/Avionics_code/blob/239cbfb9d4ad30f8ef577375a49e0b4af8b44a7f/CPP_flight_computer_program/src/main.cpp#L276-L301

frroossst commented 1 year ago
data size data type
time 04 bytes u32
rocket state 01 byte u8
acceleration x axis 02 bytes float16
acceleration y axis 02 bytes float16
acceleration z axis 02 bytes float16
gyroscope x axis 02 bytes float16
gyroscope y axis 02 bytes float16
gyroscope z axis 02 bytes float16
relative altitude 02 bytes float16
pressure 02 bytes float16
thermocouple temp 02 bytes float16
extra space 02 byte undefined
total 25 bytes

I've used collect and store interchangeably here, please note, collection here refers to collection to the fram i.e. storage to the fram

Say, this is what one sensor chunk looks like, takes up 25 bytes, 2 bytes are reserved for whatever extra data that we might need to store in the future, maybe payload info, or thermocouple internal temperature or anything really, I've kept it as 2 bytes (could be more).

Now our fram can store 32Kbyte(s) which is 32,000 bytes. now, one chunk of data is 25, so we'll be able to write $$32,000/25 = 1,280$$

So, we can write to one FRAM 1280 chunks of data. so far so good.

If we write 1 chunk per second, we'll be able to collect data for 1280 seconds which is 21.33 minutes.

But, as the preliminary testing over discord the other day suggested, our loop() executes about 500 times in every other state with the exception of the aforementioned state (~2) and ground_idle (>2000).

Now, even if I were to assume that we're collecting data 250 times a second which is half of what the expected tick rate for loop() is, we'll be able to collect 5 seconds worth of data.

Assuming we're collecting data every 100 times a second, which is 20% of the nominal loop() tick rate, we'll be able to collect 12.8 seconds worth of data

Assuming we're collecting data 2 times a second, this gives us 640 seconds worth of flight data, which is roughly 10 minutes, if we double to rate i.e. collect data 4 times every second, gives us 5 minutes or so of flight data.

It is also of essence to note, that we're talking about flight data i.e. when to rocket is in powered_flight to up until it hits ground_idle, we have to leave some "padding" as we do want to log data when the rocket is sitting on the pad and even after it has landed safely, so 5 minutes of in-flight data could very easily be translated to 10 or so minutes of actual real time data.

Now, a simple solution is to add more FRAM if you want to keep up the high tick rate of data collection cum storage, you'd need to have 60 FRAMs to store about 5 minutes worth of data, at a tick rate of 250 per second, so naturally the other solution is to slow down the tick rate, by how much you might ask? well, about 2 per second, would be my recommendation, strikes a balance between the data we collect, and the storage we have without needing to add additional storage. And this "blocking" loop accommodates that requirement with zero modifications.


So, the conclusion is I will not be modifying this specific part of the code to be non-blocking for the time being and with the current hardware limitations, as under no circumstances at least with respect to data collection can this be considered ''blocking", if someone has a better argument apart from ''we need data in the meanwhile" which as demonstrated above is a fairly smooth brained take, and "while true bad" which demonstrates a lack of understanding of how code actually works and interacts with the system at large, if your argument is "we need the data for something" then I need to see a MVP or proof of concept of that something that you're referring to, making premature optimizations for phantom somethings is NOT something we should be wasting our time on.

If you have other arguments then please leave them down below in the thread!

dhaussecker commented 1 year ago

"as under no circumstances at least with respect to data collection can this be considered blocking" With respect to data collection, this may be true it's not going to impact us, but to respect to the whole entire system, it's going to impact us long-term.

Blocking code will make it EXTREMELY difficult to expand our system in the future, and I guarantee you this if you're writing not proper code right now with your excuse being "we don't need it right now" it will have to be rewritten in the future and waste more time.

There is an easy way to fix this, just use the same implementation we are using with tracking time using millis(), to remove the blocking part of the code. This is an easy fix, it will impact a few other functions that you will have to take into account but it is better than impacting 100 functions down the road when you realize your blocking code is holding you back a year from now. If you don't get to this, I'll fix it within the next couple of weeks.

"Write proper code now, so you do not have to 1 year from now" -Dylan ;)

frroossst commented 1 year ago

I'm upset cause your whole argument was data collection :sob: :sob: :sob:

Blocking code is problematic if and only if you need to be doing something in the meanwhile, which we are not (right now), in the future perhaps, but like, what even would we be doing? We'll get to this when we do radio integration (even then we might not need to)

dhaussecker commented 1 year ago

The fix for this will take 20 minutes, yet this discussion has taken multiple hours. Let’s just fix it when we have the time

frroossst commented 1 year ago

Because it very much matters the path you take to a solution on a team project, as the "true" intentions might be masked behind a bs reason.

For example, the decision to switch from I2C to CANbus, could be justified as speed restrictions of I2C and it would be a perfectly reasonable explanation, but, the actual reasoning is length restrictions of I2C and the increased flexibility with CANbus with priority queues.

If someone is referencing these threads in the future, we're providing them with not actual truths behind our decisions, and if not for future-proofing we're propagating falsities through the team. This just creates a bad habit which'll result in something catastrophic down the line, let's say like buying a 20,000 dollar camera which does not work with the OBC 0_o

dhaussecker commented 1 year ago

Code is still blocking, removing the while true in apogee check will conics me it is not

frroossst commented 1 year ago

technical analysis of the impact or it didn't happen!