Bug 1287 - Add metadata defaults for the Logic Pirate device
Summary: Add metadata defaults for the Logic Pirate device
Status: CONFIRMED
Alias: None
Product: libsigrok
Classification: Unclassified
Component: Driver: openbench-logic-sniffer (show other bugs)
Version: unreleased development snapshot
Hardware: All All
: Normal normal
Target Milestone: ---
Assignee: Nobody
URL:
Keywords: low_hanging_fruit
Depends on:
Blocks:
 
Reported: 2018-09-24 01:13 CEST by pascal martin
Modified: 2018-09-25 10:47 CEST (History)
0 users



Attachments
Changes to the OLS driver to use device-specific metadata defaults (2.07 KB, patch)
2018-09-24 01:13 CEST, pascal martin
Details
libsigrock patch to support the Logic Pirate, version 2 (5.46 KB, patch)
2018-09-25 10:47 CEST, pascal martin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description pascal martin 2018-09-24 01:13:08 CEST
Created attachment 458 [details]
Changes to the OLS driver to use device-specific metadata defaults

I am using Debian testing, pulseview 0.4.0 and libsigrok 0.5.0.

When connecting to the SeeStudio Logic Pirate in pulseview, pulseview shows Logic Pirate as having 0 channels and exits brutally.

Using verbose mode (-l 5) it appears that the Logic Pirate does not return the number of channels, max number of samples and max sample rate metadata items expected by the OLS driver.

The code in src/hardware/openbench-logic-sniffer/protocol.c just leaves the three metadata items to 0, which pulseview finds unacceptable.

I downloaded the latest master branch of libsegrok on Sep 23, 2018:

git clone git://sigrok.org/libsigrok

I was able to capture samples from the Logic Pirate after a small change to protocol.c (see attached patch file). The change is simply to detect when the items are missing and assign to each of them a default value that depends on the name of the device (this way different defaults can be used for different devices).

This change is admittedly a bit crude. Ideally the user should be able to set his own default parameters in a configuration file. Also it does not handle the 60 MHz variant: the default always limit the max sample rate to 40 MHz.
Comment 1 pascal martin 2018-09-24 01:25:33 CEST
Please note that the Logic Pirate has a fixed number of channels and a fixed number of samples. Thus using hard-coded defaults is not by itself too much of an issue. Anyone using the 60 MHz firmware could propose a patch to handle both the 40 MHz and the 60 MHz variants; that could allow the extra 60 MHz sampling rate.

(However I am not sure that pulseview actually uses the max sample rate: selecting 50 MHz to 200 MHz is allowed, and cause an error, while 40 MHz is not proposed.. The list of rates seems to be fixed.)

Handling a user-defined file would be most useful to allow handling other (new) devices, or to adjust for new versions of the hardware and/or firmware without requiring to wait for a new release of libsigrok.
Comment 2 pascal martin 2018-09-25 10:47:40 CEST
Created attachment 459 [details]
libsigrock patch to support the Logic Pirate, version 2