Hi,
Thanks. I think I've addressed all of your points.
The only style point I really disagree with is using an instance variable
for the samplenum, but we're not multithreaded so that should be fine.
I'm not exactly sure how you would like the dump directory structure to
look so I gave it a good attempt. The common wiegand format is 26 bit, and
the bit width is a differentiator between formats, so I went with
wiegand/XX-bits/
Test passes:
sigrokdecode/sigrok-test$ ./decoder/pdtest -r -v wiegand
wiegand/wiegand_36bit/annotation
........................................... OK
On Tue, May 10, 2016 at 12:58 AM, Uwe Hermann <u...@hermann-uwe.de> wrote:
> 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
>
--
Sean Burford <sburf...@google.com>
lgtm, approval.
From b4c04300f458defa0a43c47f308a7ecc908f78d2 Mon Sep 17 00:00:00 2001
From: Sean Burford <sburf...@google.com>
Date: Tue, 10 May 2016 17:25:21 +1000
Subject: [PATCH 1/1] Style fixes based on feedback
---
decoders/wiegand/__init__.py | 29 ++++++++++
decoders/wiegand/pd.py | 134 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 163 insertions(+)
create mode 100644 decoders/wiegand/__init__.py
create mode 100644 decoders/wiegand/pd.py
diff --git a/decoders/wiegand/__init__.py b/decoders/wiegand/__init__.py
new file mode 100644
index 0000000..20f51f8
--- /dev/null
+++ b/decoders/wiegand/__init__.py
@@ -0,0 +1,29 @@
+##
+## This file is part of the libsigrokdecode project.
+##
+## Copyright (C) 2016 Sean Burford <sburf...@google.com>
+##
+## This program is free software; you can redistribute it and/or modify
+## it under the terms of the GNU General Public License as published by
+## the Free Software Foundation; either version 2 of the License, or
+## (at your option) any later version.
+##
+## This program is distributed in the hope that it will be useful,
+## but WITHOUT ANY WARRANTY; without even the implied warranty of
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+## GNU General Public License for more details.
+##
+## You should have received a copy of the GNU General Public License
+## along with this program; if not, write to the Free Software
+## Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+##
+
+'''
+The Wiegand interface is a de facto wiring standard commonly used to connect
+a card swipe mechanism to the rest of an electronic entry system.
+
+Details:
+https://en.wikipedia.org/wiki/Wiegand_interface
+'''
+
+from .pd import Decoder
diff --git a/decoders/wiegand/pd.py b/decoders/wiegand/pd.py
new file mode 100644
index 0000000..b7b19ec
--- /dev/null
+++ b/decoders/wiegand/pd.py
@@ -0,0 +1,134 @@
+##
+## This file is part of the libsigrokdecode project.
+##
+## Copyright (C) 2016 Sean Burford <sburf...@google.com>
+##
+## This program is free software; you can redistribute it and/or modify
+## it under the terms of the GNU General Public License as published by
+## the Free Software Foundation; either version 2 of the License, or
+## (at your option) any later version.
+##
+## This program is distributed in the hope that it will be useful,
+## but WITHOUT ANY WARRANTY; without even the implied warranty of
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+## GNU General Public License for more details.
+##
+## You should have received a copy of the GNU General Public License
+## along with this program; if not, write to the Free Software
+## Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+##
+
+import sigrokdecode as srd
+
+class Decoder(srd.Decoder):
+ api_version = 2
+ id = 'wiegand'
+ name = 'Wiegand'
+ longname = 'Wiegand interface'
+ desc = 'Wiegand interface for electronic entry systems.'
+ license = 'gplv2+'
+ inputs = ['logic']
+ outputs = ['wiegand']
+ channels = (
+ {'id': 'd0', 'name': 'D0', 'desc': 'Data 0 line'},
+ {'id': 'd1', 'name': 'D1', 'desc': 'Data 1 line'},
+ )
+ options = (
+ {'id': 'active', 'desc': 'Data lines active level',
+ 'default': 'low', 'values': ('low', 'high')},
+ {'id': 'bitwidth_ms', 'desc': 'Single bit width in milliseconds',
+ 'default': '4', 'values': ('1', '2', '4', '8', '16', '32')},
+ )
+ annotations = (
+ ('bits', 'Bits'),
+ ('state', 'State'),
+ )
+ annotation_rows = (
+ ('bits', 'Binary Value', (0,)),
+ ('state', 'Stream State', (1,)),
+ )
+
+ def __init__(self, **kwargs):
+ self._samples_per_bit = 10
+
+ self._d0_prev = None
+ self._d1_prev = None
+
+ self._state = None
+ self._ss_state = None
+
+ self.ss_bit = None
+ self.es_bit = None
+ self._bit = None
+ self._bits = []
+
+ def start(self):
+ 'Register output types and verify user supplied decoder values.'
+ self.out_ann = self.register(srd.OUTPUT_ANN)
+ self._active = self.options['active'] == 'high' and 1 or 0
+ self._inactive = 1 - self._active
+
+ def metadata(self, key, value):
+ 'Receive decoder metadata about the data stream.'
+ if key == srd.SRD_CONF_SAMPLERATE:
+ ms_per_sample = 1000 * (1.0 / value)
+ ms_per_bit = float(self.options['bitwidth_ms'])
+ self._samples_per_bit = int(max(1, int(ms_per_bit / ms_per_sample)))
+
+ def _update_state(self, 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.ss_bit, self.samplenum, self.out_ann,
+ [0, [str(self._bit),]])
+ self._bit = bit
+ self.ss_bit = self.samplenum
+ if bit is not None:
+ # Set a timeout so that the final bit ends.
+ self.es_bit = self.samplenum + self._samples_per_bit
+ else:
+ self.es_bit = None
+
+ if state != self._state:
+ ann = None
+ if self._state == 'data':
+ accum_bits = ''.join(str(x) for x in self._bits)
+ ann = [1, ['%d bits %s' % (len(self._bits), accum_bits),
+ '%d bits' % len(self._bits)]]
+ elif self._state == 'invalid':
+ ann = [1, [self._state]]
+ if ann:
+ self.put(self._ss_state, self.samplenum, self.out_ann, ann)
+ self._ss_state = self.samplenum
+ self._state = state
+ self._bits = []
+
+ def decode(self, ss, es, data):
+ for self.samplenum, (d0, d1) in data:
+ if d0 == self._d0_prev and d1 == self._d1_prev:
+ if self.es_bit and self.samplenum >= self.es_bit:
+ if (d0, d1) == (self._inactive, self._inactive):
+ self._update_state('idle')
+ else:
+ self._update_state('invalid')
+ continue
+
+ if self._state in (None, 'idle', 'data'):
+ if (d0, d1) == (self._active, self._inactive):
+ self._update_state('data', 0)
+ elif (d0, d1) == (self._inactive, self._active):
+ self._update_state('data', 1)
+ elif (d0, d1) == (self._active, self._active):
+ self._update_state('invalid')
+ elif self._state == 'invalid':
+ # Wait until we see an idle state before leaving invalid.
+ # This prevents inverted lines from being misread.
+ if (d0, d1) == (self._inactive, self._inactive):
+ self._update_state('idle')
+
+ self._d0_prev, self._d1_prev = d0, d1
+
+ 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)
--
2.8.0.rc3.226.g39d4020
From a36e7036a35f24ebffe7bf5edd374739426374e7 Mon Sep 17 00:00:00 2001
From: Sean Burford <sburf...@google.com>
Date: Tue, 10 May 2016 18:29:43 +1000
Subject: [PATCH 2/2] Add wiegand/36bit dump.
---
wiegand/36bit/README | 25 +++++++++++++++++++++++++
wiegand/36bit/wiegand-8A640910.sr | Bin 0 -> 1427 bytes
2 files changed, 25 insertions(+)
create mode 100644 wiegand/36bit/README
create mode 100644 wiegand/36bit/wiegand-8A640910.sr
diff --git a/wiegand/36bit/README b/wiegand/36bit/README
new file mode 100644
index 0000000..216386d
--- /dev/null
+++ b/wiegand/36bit/README
@@ -0,0 +1,25 @@
+------------------------------------------------------------------------------
+Wiegand connected RFID reader captures
+-------------------------------------------------------------------------------
+
+This is a set of example captures of Wiegand traffic from RFID reader modules.
+
+Details:
+https://en.wikipedia.org/wiki/Wiegand_interface
+
+Logic analyzer setup
+--------------------
+
+The logic analyzer used was a DSLogic Pro (20kHz sample rate) connected to a
+generic brand 13.56MHz reader with 36 bit output enabled firmware.
+
+ Probe Reader
+ ------------------
+ 0 D0
+ 1 D1
+
+
+Data
+----
+
+The RFID tag hex values are encoded in the respective filenames.
diff --git a/wiegand/36bit/wiegand-8A640910.sr b/wiegand/36bit/wiegand-8A640910.sr
new file mode 100644
index 0000000000000000000000000000000000000000..6f9d5c85f5ed0636f6c7e3abb1dc509639464219
GIT binary patch
literal 1427
zcmWIWW@Zs#U|`^22o9O;!MpFO6f=;=2*m6_T$Wl?oSC0z%)}6YqOM4L=Cnm1T|k8#
zKsq<IBrzqiBvJRAFJH5QK-<H=a}IG|G0^&_v#<7jLd=4tZ64i>B@bU_bM022`Q&d}
z%-Y#g=W5Kjx_hm~R{mqPil<hIxIFfrpLUVef6EG6xw!qkS2yIDo^ICMwIXL@wfSp?
zJk1s7^R|WuE*3iaZO@DP<c=1uIRT5kG6K{+OgME@oZ8J6x=%M)aNe;|vHPIqxpLd*
zJK3Hy<{hYPQnlzW;GR}e<ID4G*ZigOsQ&n5crs85=zazT1_n+bm6M;InXGH5YxwpM
zBZ$wmV8Q?Sw*@=QSb-c6ILAwz0Y;P>U`(t5f6qRe?F4d~0;(5G-d-`}V-OH<b<BQW
zpKf}QQ&H{m|3|4OC+Vzco_=ld=IuRSE5BC!NIDd472@_?`qYoZ=hp0YPp#8>+`fem
zWS+GNab7hg)}`rdcl|X5nXG{7QnR<04Y?Q;c$giw{{O$Z+PvLK`}kW$79nkrS_Xz6
zCp^`bzCU7AR{uKg_p->BTTfSHo!5^=b=+}H`Ja{`)e5L)o4>u{2sE36;ozzN|94g&
zKXFLn*}jgBi%GX5js(8Se^ootvU<v@^mj`?rTyFKUjHCW7GwxRgN!h$`M45_1wPL+
zGBGgV&RbxM-~=?M1$d)sLeFS$^$Z$7CM?^b>q2)eLe~c%6Q0P4*Jj9w&}Kxuwjv~L
z#^~D66E4D$n~}5u6CrAd;P#sgFt}lUGey^ip7;>9#Ug1lL)V6$t`ORGB55;6*M{4X
clEC5>=17YGZ&o&tiCjRa3^XqaRQ56e0NJ)==l}o!
literal 0
HcmV?d00001
--
2.8.0.rc3.226.g39d4020
From 80f2832009d81b34fbdd0708cba8655af5d98c9e Mon Sep 17 00:00:00 2001
From: Sean Burford <sburf...@google.com>
Date: Tue, 10 May 2016 18:30:08 +1000
Subject: [PATCH 3/3] Add wiegand 36bit test
---
decoder/test/wiegand/test.conf | 5 ++
decoder/test/wiegand/wiegand-8A640910.output | 70 ++++++++++++++++++++++++++++
2 files changed, 75 insertions(+)
create mode 100644 decoder/test/wiegand/test.conf
create mode 100644 decoder/test/wiegand/wiegand-8A640910.output
diff --git a/decoder/test/wiegand/test.conf b/decoder/test/wiegand/test.conf
new file mode 100644
index 0000000..e11ae15
--- /dev/null
+++ b/decoder/test/wiegand/test.conf
@@ -0,0 +1,5 @@
+test wiegand_36bit
+ protocol-decoder wiegand channel d0=0 channel d1=1
+ input wiegand/36bit/wiegand-8A640910.sr
+ output wiegand annotation match wiegand-8A640910.output
+
diff --git a/decoder/test/wiegand/wiegand-8A640910.output b/decoder/test/wiegand/wiegand-8A640910.output
new file mode 100644
index 0000000..f48b2c4
--- /dev/null
+++ b/decoder/test/wiegand/wiegand-8A640910.output
@@ -0,0 +1,70 @@
+12448-12495 wiegand: bits: "0"
+12495-12542 wiegand: bits: "0"
+12542-12589 wiegand: bits: "1"
+12589-12636 wiegand: bits: "0"
+12636-12683 wiegand: bits: "0"
+12683-12729 wiegand: bits: "0"
+12729-12776 wiegand: bits: "1"
+12776-12823 wiegand: bits: "0"
+12823-12870 wiegand: bits: "1"
+12870-12917 wiegand: bits: "0"
+12917-12964 wiegand: bits: "0"
+12964-13011 wiegand: bits: "1"
+13011-13058 wiegand: bits: "1"
+13058-13105 wiegand: bits: "0"
+13105-13152 wiegand: bits: "0"
+13152-13199 wiegand: bits: "1"
+13199-13246 wiegand: bits: "0"
+13246-13293 wiegand: bits: "0"
+13293-13340 wiegand: bits: "0"
+13340-13386 wiegand: bits: "0"
+13386-13433 wiegand: bits: "0"
+13433-13480 wiegand: bits: "0"
+13480-13527 wiegand: bits: "1"
+13527-13574 wiegand: bits: "0"
+13574-13621 wiegand: bits: "0"
+13621-13668 wiegand: bits: "1"
+13668-13715 wiegand: bits: "0"
+13715-13762 wiegand: bits: "0"
+13762-13809 wiegand: bits: "0"
+13809-13856 wiegand: bits: "1"
+13856-13903 wiegand: bits: "0"
+13903-13950 wiegand: bits: "0"
+13950-13997 wiegand: bits: "0"
+13997-14077 wiegand: bits: "0"
+12448-14077 wiegand: state: "34 bits 0010001010011001000000100100010000" "34 bits"
+23808-23855 wiegand: bits: "0"
+23855-23902 wiegand: bits: "0"
+23902-23949 wiegand: bits: "1"
+23949-23996 wiegand: bits: "0"
+23996-24043 wiegand: bits: "0"
+24043-24090 wiegand: bits: "0"
+24090-24137 wiegand: bits: "1"
+24137-24184 wiegand: bits: "0"
+24184-24231 wiegand: bits: "1"
+24231-24278 wiegand: bits: "0"
+24278-24325 wiegand: bits: "0"
+24325-24372 wiegand: bits: "1"
+24372-24418 wiegand: bits: "1"
+24418-24465 wiegand: bits: "0"
+24465-24512 wiegand: bits: "0"
+24512-24559 wiegand: bits: "1"
+24559-24606 wiegand: bits: "0"
+24606-24653 wiegand: bits: "0"
+24653-24700 wiegand: bits: "0"
+24700-24747 wiegand: bits: "0"
+24747-24794 wiegand: bits: "0"
+24794-24841 wiegand: bits: "0"
+24841-24888 wiegand: bits: "1"
+24888-24935 wiegand: bits: "0"
+24935-24982 wiegand: bits: "0"
+24982-25029 wiegand: bits: "1"
+25029-25076 wiegand: bits: "0"
+25076-25122 wiegand: bits: "0"
+25122-25169 wiegand: bits: "0"
+25169-25216 wiegand: bits: "1"
+25216-25263 wiegand: bits: "0"
+25263-25310 wiegand: bits: "0"
+25310-25357 wiegand: bits: "0"
+25357-25437 wiegand: bits: "0"
+23808-25437 wiegand: state: "34 bits 0010001010011001000000100100010000" "34 bits"
--
2.8.0.rc3.226.g39d4020
------------------------------------------------------------------------------
Mobile security can be enabling, not merely restricting. Employees who
bring their own devices (BYOD) to work are irked by the imposition of MDM
restrictions. Mobile Device Manager Plus allows you to control only the
apps on BYO-devices by containerizing them, leaving personal data untouched!
https://ad.doubleclick.net/ddm/clk/304595813;131938128;j
_______________________________________________
sigrok-devel mailing list
sigrok-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sigrok-devel
