Hi, On Thu, Aug 27, 2015 at 11:21:49AM +0200, Daniel Glöckner wrote: > The Hung-Chang DSO-2100 is a parallel port PC oscilloscope sold back > in 1999 under brand names like Protek and Voltcraft.
Thanks a lot for writing the driver! As per IRC discussion, could you please indeed add your changeѕ on top of the 'new-driver' generated stubs, and split the code into api.c and protocol.[ch] as in the other drivers? Thanks! > +#include <ieee1284.h> > +#include <string.h> > +#include <glib.h> > +#include <libsigrok/libsigrok.h> > +#include "libsigrok-internal.h" > + > +#define LOG_PREFIX "hung-chang-dso-2100" > + > +/* The firmware can be in the following states: > + * 0x00 Temporary state during initializazion initialization > + * Automatically transitions to state 0x01 > + * 0x01 Idle, this state updates calibration caps > + * Send 0x02 to go to state 0x21 > + * Send 0x03 to go to state 0x03 > + * Send 0x04 to go to state 0x14 > + * 0x21 Trigger is armed, caps are _not_ updated > + * Send 0x99 to check if trigger event occured > + * if triggered, goes to state 0x03 > + * else stays in state 0x21 > + * Send 0xFE to generate artifical trigger event artificial > + * returns to state 0x21 > + * but next 0x99 will succeed > + * Send 0xFF to go to state 0x03 (abort capture) > + * 0x03 Extracts two 500 sample subsets from the 5000 > + * sample capture buffer for readout > + * When reading samples, the FPGA starts at the > + * first of the 1000 samples an automatically and > + for (i = 0; i < 2; i++) { Maybe add NUM_CHANNELS for this 2 for slightly more readable code. > +#ifndef SCAN_ALL_PARALLEL_PORTS > + if (!conn) > + return NULL; > +#endif Hm, this should probably go, blindly scanning parallel ports should be avoided, we don't know which devices are connected and how they (mis)react to data sent to them (same reason why we don't scan on serial ports either). > +static int config_channel_set(const struct sr_dev_inst *sdi, > + struct sr_channel *channel, Please use "ch" as name for "struct sr_channel *" items to be a bit more consistent with the rest of the code-base (and "cg" for channel groups). > +static void dso2100_skip_samples(struct parport *port, uint8_t ctrl, size_t > num) > +{ > + while(num--) { ^ missing space here > + if (buf[1000] == 0x01 && buf[1001] == 0xfe && buf[1002] > == 0x80) { > + if (!offset) { > + push_samples(sdi, buf, 6); > + packet.type = SR_DF_TRIGGER; The indentation doesn't look correct here. Apart from the minor stuff I noticed in the quick review above, the driver looks really nice and clean. Great job! Uwe. -- http://hermann-uwe.de | http://randomprojects.org | http://sigrok.org ------------------------------------------------------------------------------ _______________________________________________ sigrok-devel mailing list sigrok-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/sigrok-devel