
Hi Tim!
The current logic expects a reset gpio and bails out if it cannot get it with a (questionable) goto statement.
You want to invert that logic, and expect no gpio, but only if there is one you want to warn.
This is perfectly fine but the logic here must be clarified. I'd go for:
ret = gpio_request() if (ret) { ret = gpio_request() if (!ret) warn(deprecated) }
if (!ret) { warn(dangerous) toggle_value() }
I would ideally replace the 'if (ret)' clauses with 'if (!reset_gpio)' which would make the checks even clearer.
Fair enough. But the code with the proposed change has no functional problems right?
No, this is functionally right, but the code is not clear like that.
If so I'll send a PR to Tom as is and rework it as suggested later
Well, that's not how contribution work usually. Is there an emergency in getting this merged?
Not really, it's a print message. But I don't currently have time to pick this up. Tim, would you mind reworking it as Miquel suggested?
I'm just catching up after being out of the office for a couple of weeks - I'll rework it and submit another revision as soon as I have some time.
Thanks! /Ilias
Best Regards,
Tim