Hi,

On Wed, Apr 27, 2016 at 10:51:37AM +1000, Sean Burford wrote:
> I've put together a basic Wiegand decoder that extracts bit streams from
> wiegand signals.  It stops at the binary level and doesn't extract facility
> codes/identity numbers etc.  This is on the list of possible future
> decoders at
> https://sigrok.org/wiki/Protocol_decoders#Possible_candidates_for_future_protocol_decoders
> 
> Decoding the attached wiegand.srzip should result in two decodes of the 36
> bit value 0010001010011001000000100100010000
> 
> Let me know if you need anything more.

Looks great, thanks!

A few minor nitpicks below:

 
> +class Decoder(srd.Decoder):
> +    api_version = 2
> +    id = 'wiegand'
> +    name = 'Wiegand'
> +    longname = 'Wiegand Interface'
> +    desc = 'Wiegand Interface for electronic entry systems.'

"Interface" -> "interface".


> +    binary = ()

Can be dropped, it's empty (to be consistent with all other PDs).


> +    def __init__(self, **kwargs):
> +        super(Decoder, self).__init__(**kwargs)

The super() call can be dropped. It has no effect and all other PDs
don't call it either.


> +        self._out_annotation = None

This is unused and can be dropped. If you meant self.out_annotation
that's not required to be set to None here.


> +    def start(self):
> +        """Register output types and verify user supplied decoder values."""

Please use single-quotes everywhere for consistency across all PDs.


> +        self.out_annotation = self.register(srd.OUTPUT_ANN)

A few more naming consistency things:

 - self.out_annotation -> self.out_ann

 - self._bit_start -> self.ss_bit
 - self._bit_end -> self.es_bit
   ("ss" is "start sample", es is "end sample", which we use
    consistently across all PDs)


> +    def _update_state(self, samplenum, state, bit=None):
> +        """Update state and bit values when they change."""
> +        if self._bit is not None:
> +            self._bits.append(self._bit)
> +            self.put(self._bit_start, samplenum, self.out_annotation,

Make samplenum -> self.samplenum please (and no need to pass it to
methods then, too). This is not relevant right now, but an upcoming
backend change will be easier to do if all PDs use self.samplenum
consistently.


> +    def decode(self, startsample, endsample, data):

 - startsample -> ss
 - endsample -> es


> +        """Receive and decode samples."""

Can be omitted IMHO, pretty obvious.


> +        for samplenum, (d0, d1) in data:

self.samplenum please, see above.


> +    def report(self):
> +        return '%s: %s D0 %d D1 %d (active on %d), %d samples per bit' % (
> +            self.name, self._state, self._d0_prev, self._d1_prev,
> +            self._active, self._samples_per_bit)
> +

Last line in pd.py should not be an empty line, just remove it please.


Can you please send wiegand.sr as patch against the sigrok-dumps repo?
Put it in a subdirectory of a new "wiegand" directory if possible, and
name the subdirectory+file(s) according to what exactly this .sr file
contains (more .sr files with different settings/data are always good to
have as well).
Other people will likely contribute other Wiegand dumps later and those
should go into other subdirs with different name etc.

A tiny README as to what's contained in the .sr file (which probes,
samplerate, LA used, data contained, which board/device was probed, etc.
etc.) would be nice as well.

Examples: rfid/fk4100/rdm630/README, others

And finally, if you have some spare time, having a set of test-cases in
our sigrok-test repo for this new decoder would be neat as well.

Example: decoder/test/am230x/test.conf, decoder/test/am230x/*.output


Thanks! Uwe.
-- 
http://hermann-uwe.de | http://randomprojects.org | http://sigrok.org

------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
sigrok-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/sigrok-devel

Reply via email to