You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The current code has major issues that make it nearly unusable for anything reliable or production-grade. Honestly, it’s hard to believe this is the code shipped with a product. I want to point out some of the most serious problems.
First off, you're using ctypes to kill threads like this is a normal thing to do:
Not only is killing threads this way dangerous, but it can also leave hardware in unpredictable states or cause memory corruption. This is not how threads should be stopped in Python. There are much safer ways to handle this, like threading.Event, which would let threads exit cleanly rather than being terminated abruptly.
Next, the use of comparison operators instead of assignments is just baffling:
self.balance_flag==False
That line doesn’t actually do anything. What you probably meant to write was self.balance_flag = False, but with ==, it just checks a condition and then moves on. No change happens, and no one notices, until the bug pops up down the line. For something controlling actual hardware, this is borderline reckless.
But the worst part might be this:
try:
stop_thread(thread_power)
except:
pass
There is no thread_power. That variable literally doesn’t exist. It’s like writing abrakadabra and hoping it works. And even if it did exist, we’d still have the same unsafe thread termination issue I mentioned earlier. But come on. You’ve got code here that doesn’t even run, and it’s obvious no one even tested this.
There’s also the overuse of magic numbers scattered throughout the code, making it nearly impossible to know what's really going on:
foriinrange(450, 89, -self.speed):
What does 450 represent? Why is 89 the cutoff? Without any explanation or comments, anyone trying to work with this code is left guessing. Magic numbers like these should at least be named constants or, even better, loaded from a configuration.
Oh, and please stop doing this:
from*import*
This is terrible practice. It makes code harder to read, harder to track where things are coming from, and generally causes more problems than it solves. Stick to importing only what you need or use a namespace.
Now to sum up the real-world problems this code causes:
Undefined variables (like thread_power) and logic errors (such as using == instead of =) lead to unexpected behavior, or worse - silent bugs you won’t notice until things start failing at random.
Abrupt thread termination could crash the hardware or leave it in unreliable states.
Magic numbers littered everywhere make the system impossible to maintain or modify.
What needs to change:
Stop using ctypes to kill threads. Use proper thread termination with threading.Event.
Fix the blatant errors with == where = should be used, and get rid of undefined variables like thread_power.
Remove magic numbers, or at least replace them with named constants or configuration parameters.
Add basic exception handling. Don't just write except: pass, because then you'll never see useful error messages for debugging.
Perform static code linting using tools like pyright, flake8 or pylint.
Honestly, it feels like you shipped this without running even the most basic tests. People paid for this, and they should at least get code that has been checked to work correctly.
The text was updated successfully, but these errors were encountered:
Thank you very much for your guidance, I will spend some time checking the code of the Raspberry PI series and do my best to make the code more normative. Even if it's not a product I designed, I take the time to optimize it as much as possible.
The current code has major issues that make it nearly unusable for anything reliable or production-grade. Honestly, it’s hard to believe this is the code shipped with a product. I want to point out some of the most serious problems.
First off, you're using
ctypes
to kill threads like this is a normal thing to do:Not only is killing threads this way dangerous, but it can also leave hardware in unpredictable states or cause memory corruption. This is not how threads should be stopped in Python. There are much safer ways to handle this, like
threading.Event
, which would let threads exit cleanly rather than being terminated abruptly.Next, the use of comparison operators instead of assignments is just baffling:
That line doesn’t actually do anything. What you probably meant to write was
self.balance_flag = False
, but with==
, it just checks a condition and then moves on. No change happens, and no one notices, until the bug pops up down the line. For something controlling actual hardware, this is borderline reckless.But the worst part might be this:
There is no
thread_power
. That variable literally doesn’t exist. It’s like writingabrakadabra
and hoping it works. And even if it did exist, we’d still have the same unsafe thread termination issue I mentioned earlier. But come on. You’ve got code here that doesn’t even run, and it’s obvious no one even tested this.There’s also the overuse of magic numbers scattered throughout the code, making it nearly impossible to know what's really going on:
What does
450
represent? Why is89
the cutoff? Without any explanation or comments, anyone trying to work with this code is left guessing. Magic numbers like these should at least be named constants or, even better, loaded from a configuration.Oh, and please stop doing this:
This is terrible practice. It makes code harder to read, harder to track where things are coming from, and generally causes more problems than it solves. Stick to importing only what you need or use a namespace.
Now to sum up the real-world problems this code causes:
thread_power
) and logic errors (such as using==
instead of=
) lead to unexpected behavior, or worse - silent bugs you won’t notice until things start failing at random.What needs to change:
ctypes
to kill threads. Use proper thread termination withthreading.Event
.==
where=
should be used, and get rid of undefined variables likethread_power
.except: pass
, because then you'll never see useful error messages for debugging.pyright
,flake8
orpylint
.Honestly, it feels like you shipped this without running even the most basic tests. People paid for this, and they should at least get code that has been checked to work correctly.
The text was updated successfully, but these errors were encountered: