On Sat, 2019-10-12 at 20:13 +0200, Benedikt Otto wrote:
>
> Thank you very much for the constructive criticism. That is the first
> time that I contribute to such a project.
>
> So far I think I fixed all the issues you mentioned. The repositories
> can be found at:
>
> https://github.com/BenediktO/libsigrokdecode.git
>
> https://github.com/BenediktO/sigrok-test.git
>
> https://github.com/BenediktO/sigrok-dumps.git
>
>
> I hope that the decoder now meets the conventions.
That was quick! :) Just looked over the most recent commits of
the PD, haven't run it yet nor looked at dumps/test updates.
Looks better now, could be close to being acceptable for
mainline. B-] Squashing the accumulated commits can be done late.
Some whitespace is left that's easy to cleanup.
Forgot to mention the last time: Don't check for equality of None
("!="), use "is" instead with an optional "not". The "!=" result
may not be what you expect.
Notice that the "dp" input signal is optional. You may have to
consider this when you create the .wait() condition (again,
haven't checked myself what happens if you specify conditions for
absent optional channels -- but it's always better to be safe
than sorry). Maybe look at other decoders which support optional
channels, the base class has methods to check for their presence.
Inverting pin levels may need similar attention, because [7] need
not always be present. Does your test data cover optional input
being present as well as absent?
Nit: Initialize "lastpos" before the loop from self.samplenum
since at this spot you already returned from the first .wait()?
Put the pins and pos assignments close to each other since they
are related.
Other than that, looks good to me. Especially if it works as
advertised. :)
virtually yours
Gerhard Sittig
--
If you don't understand or are scared by any of the above
ask your parents or an adult to help you.
_______________________________________________
sigrok-devel mailing list
sigrok-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sigrok-devel
