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

Reply via email to