Bug 1024 - Decoder channel assignment issues
Summary: Decoder channel assignment issues
Status: RESOLVED FIXED
Alias: None
Product: PulseView
Classification: Unclassified
Component: Protocol decoding (show other bugs)
Version: unreleased development snapshot
Hardware: All All
: Normal normal
Target Milestone: ---
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-06 00:54 CEST by Uwe Hermann
Modified: 2017-09-08 22:40 CEST (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Uwe Hermann 2017-09-06 00:54:35 CEST
How to reproduce:

 - Load sigrok-dumps/uart/hello_world/8n1/hello_world_8n1_115200.sr.
 - There's one TX channel (index 0 in the .sr file) in that file, no RX (though there are other channels in the file, name and index 1-7, they're just hidden by default by PV).
 - Add a UART PD.
 - PV auto-assigns (judging from the UI widget) the TX channel (index 0 in the .sr file) to the uart PD's "TX" decoder channel, which is correct.
 - In the -l 5 output it becomes clear that the channel assignment passed to libsigrokdecode doesn't appear to be correct, though:

srd: Setting channels for instance uart-1 with list of 2 channels.
srd: Setting channel mapping: tx (PD ch idx 1) = input data ch idx 1.
srd: Setting channel mapping: rx (PD ch idx 0) = input data ch idx 0.
srd: Final channel map:
srd:  - PD ch idx 0 (rx) = input data ch idx 0 (optional)
srd:  - PD ch idx 1 (tx) = input data ch idx 1 (optional)

The expected result (e.g. from sigrok-cli) looks like this:

$ sigrok-cli -i hello_world_8n1_115200.sr -P uart:tx=TX -l 5
srd: Setting channels for instance uart-1 with list of 1 channels.
srd: Setting channel mapping: tx (PD ch idx 1) = input data ch idx 0.
srd: Final channel map:
srd:  - PD ch idx 0 (rx) = input data ch idx -1 (optional)
srd:  - PD ch idx 1 (tx) = input data ch idx 0 (optional)


Another example:

 - Open sigrok-dumps/spi/wordwidths/152bit_spi.sr
 - PV auto-assigns all 4 channels:

srd: Setting channels for instance spi-1 with list of 4 channels.
srd: Setting channel mapping: mosi (PD ch idx 2) = input data ch idx 2.
srd: Setting channel mapping: miso (PD ch idx 1) = input data ch idx 1.
srd: Setting channel mapping: clk (PD ch idx 0) = input data ch idx 0.
srd: Setting channel mapping: cs (PD ch idx 3) = input data ch idx 3.
srd: Final channel map:
srd:  - PD ch idx 0 (clk) = input data ch idx 0 (required)
srd:  - PD ch idx 1 (miso) = input data ch idx 1 (optional)
srd:  - PD ch idx 2 (mosi) = input data ch idx 2 (optional)
srd:  - PD ch idx 3 (cs) = input data ch idx 3 (optional)

I believe this is incorrect for (another) reason: "PD ch idx 2 (mosi) = input data ch idx 2" should probably be "PD ch idx 2 (mosi) = input data ch idx 1" since PV's channel with index 1 is "MOSI" (?) Assuming that PV's internal data indices correspond to the order in which the logic traces are ordered by default (before the user reshuffles them potentially). 


Anyway. Now remove the MOSI line from the PV decoder setup. The expected result is that the MOSI channel is not supplied to the PD and it thus only shows the MISO annotations. In reality it still shows both MISO and MOSI annotations.

Log output (same as above):

srd: Setting channels for instance spi-1 with list of 4 channels.
srd: Setting channel mapping: mosi (PD ch idx 2) = input data ch idx 2.
srd: Setting channel mapping: miso (PD ch idx 1) = input data ch idx 1.
srd: Setting channel mapping: clk (PD ch idx 0) = input data ch idx 0.
srd: Setting channel mapping: cs (PD ch idx 3) = input data ch idx 3.
srd: Final channel map:
srd:  - PD ch idx 0 (clk) = input data ch idx 0 (required)
srd:  - PD ch idx 1 (miso) = input data ch idx 1 (optional)
srd:  - PD ch idx 2 (mosi) = input data ch idx 2 (optional)
srd:  - PD ch idx 3 (cs) = input data ch idx 3 (optional)


For comparison, sigrok-cli with all 4 channels:

$ sigrok-cli -i 152bit_spi.sr -P spi:clk=CLK:miso=MISO:mosi=MOSI:cs='CS#'
srd: Setting channel mapping: mosi (PD ch idx 2) = input data ch idx 8.
srd: Setting channel mapping: miso (PD ch idx 1) = input data ch idx 9.
srd: Setting channel mapping: clk (PD ch idx 0) = input data ch idx 7.
srd: Setting channel mapping: cs (PD ch idx 3) = input data ch idx 12.
srd: Final channel map:
srd:  - PD ch idx 0 (clk) = input data ch idx 7 (required)
srd:  - PD ch idx 1 (miso) = input data ch idx 9 (optional)
srd:  - PD ch idx 2 (mosi) = input data ch idx 8 (optional)
srd:  - PD ch idx 3 (cs) = input data ch idx 12 (optional)

sigrok-cli with only 3 channels:

$ sigrok-cli -i 152bit_spi.sr -P spi:clk=CLK:miso=MISO:cs='CS#'
srd: Setting channels for instance spi-1 with list of 3 channels.
srd: Setting channel mapping: miso (PD ch idx 1) = input data ch idx 9.
srd: Setting channel mapping: clk (PD ch idx 0) = input data ch idx 7.
srd: Setting channel mapping: cs (PD ch idx 3) = input data ch idx 12.
srd: Final channel map:
srd:  - PD ch idx 0 (clk) = input data ch idx 7 (required)
srd:  - PD ch idx 1 (miso) = input data ch idx 9 (optional)
srd:  - PD ch idx 2 (mosi) = input data ch idx -1 (optional)
srd:  - PD ch idx 3 (cs) = input data ch idx 12 (optional)


Another issue that can be observed with this file is that the indices passed to srd are different between sigrok-cli and PulseView.

PV has clk=channel0, sigrok-cli has clk=channel7. In PV you can click "Enable all" to see all potentially available channels from the .sr file. CLK is indeed channel index 7 within the file, i.e. bit 7 in the 16-bit samples.

However, the decoding seems to work fine in PV, probably because it doesn't pass the unmodified samples from the .sr file to srd, but rather an "optimized" PV-internal version with changed indices(?) If so, there's no bug per se with that, but one needs to be careful when comparing PV indices to sigrok-cli indices then.
Comment 2 Uwe Hermann 2017-09-08 22:40:59 CEST
Fixed in 6e7a4a0066c15d99c891765bbc6797d339ac0ec8, thanks!