hd-zero / hdzero-goggle

MIT License
261 stars 76 forks source link

Simplify post bootup action handling #457

Closed Master92 closed 2 days ago

Master92 commented 1 week ago

I got confused about how the post bootup actions were handled so I decided to change the approach of how the software deals with them.

First off, I could reduce the necessary array size of the bootup actions by using the stdlib qsort method instead of a bubble sort approach. Secondly, instead of querying all possibly existing post bootup actions when updating the menu, I start the first action and then call the handling method again where the next action is already selected as soon as the action completes. This even works with multithreading because every action is only dispatched from the main ui thread.

SumolX commented 1 week ago

After going over this code multiple times.... while I am not a huge fan of the callback_complete and dirtying up the function signatures which are used in other places and now contain NULL arguments.... overall there are less lines of code which means less potential bugs in the future.

Master92 commented 1 week ago

Thank you for your review and feedback, I very much appreciate that.

I agree about the change of the method signature and will go another route. I tried to wrap my head around another way of acomplishing the current behavior without the callback and without having to go through the whole array all the time but I couldn't think of a better solution yet.

One idea was using a pthread_mutex_t and passing that around, having each thread to lock it as the first step but I'm not very sure if that would keep the execution order when more asynchronous post bootup actions appear.

If you have a better idea, I'm open to suggestions 😉

Master92 commented 1 week ago

@SumolX I just had an idea: how about I add a new method for handling the post bootup actions for each the wifi and storage pages that in turn only execute the existing methods plus an optional callback? That way, I get to finish my callback approach and everyone else keeps their existing method signatures without unnecessary NULL arguments!

SumolX commented 6 days ago

@SumolX I just had an idea: how about I add a new method for handling the post bootup actions for each the wifi and storage pages that in turn only execute the existing methods plus an optional callback? That way, I get to finish my callback approach and everyone else keeps their existing method signatures without unnecessary NULL arguments!

That sounds great as it would be dedicated interface for that specific functionality and will be easy to read and maintain.

SumolX commented 5 days ago

Looks great but I haven't had time to test this. Perhaps later tonight I can try it out otherwise let's get these modifications merged.