From 6e7a4a0066c15d99c891765bbc6797d339ac0ec8 Mon Sep 17 00:00:00 2001 From: Soeren Apel Date: Fri, 8 Sep 2017 18:14:47 +0200 Subject: [PATCH] Fix #1024 by changing decode channel assigment to PDs When assigning the decoder stack channels to the libsrd instance's channels, channels that had no signal assigned to them were still assigned anyway. This patch fixes this bug. After doing this, another subtle bug became apparent: The mapping between channels and bits in the data stream sent to the PD was done via DecodeChannel->id. This is however insufficient as the channels of the decoder stack have IDs that may or may not match the ID needed for the data stream. Example: A PD has 4 channels: A, B, C and D. In PV, those channels have the IDs 0, 1, 2 and 3. If the user only assigns A and D, Decoder::create_decoder_inst() will use the IDs 0 and 3 as the bit positions of those signals in the data stream sent to libsrd. This is obviously wrong. Hence, we now use a separate bit_id for this purpose. --- pv/data/decode/decoder.cpp | 7 +++++-- pv/data/decodesignal.cpp | 7 +++++-- pv/data/decodesignal.hpp | 3 ++- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/pv/data/decode/decoder.cpp b/pv/data/decode/decoder.cpp index 511f7bff..1c941074 100644 --- a/pv/data/decode/decoder.cpp +++ b/pv/data/decode/decoder.cpp @@ -122,11 +122,14 @@ srd_decoder_inst* Decoder::create_decoder_inst(srd_session *session) const g_str_equal, g_free, (GDestroyNotify)g_variant_unref); for (DecodeChannel *ch : channels_) { + if (!ch->assigned_signal) + continue; + init_pin_states->data[ch->id] = ch->initial_pin_state; - GVariant *const gvar = g_variant_new_int32(ch->id); // id = bit position + GVariant *const gvar = g_variant_new_int32(ch->bit_id); // bit_id = bit position g_variant_ref_sink(gvar); - // key is channel name, value is bit position in each sample + // key is channel name (pdch->id), value is bit position in each sample (gvar) g_hash_table_insert(channels, ch->pdch_->id, gvar); } diff --git a/pv/data/decodesignal.cpp b/pv/data/decodesignal.cpp index daa52ed6..6aa3f42f 100644 --- a/pv/data/decodesignal.cpp +++ b/pv/data/decodesignal.cpp @@ -574,7 +574,7 @@ void DecodeSignal::update_channel_list() if (!ch_added) { // Create new entry without a mapped signal - data::DecodeChannel ch = {id++, false, nullptr, + data::DecodeChannel ch = {id++, 0, false, nullptr, QString::fromUtf8(pdch->name), QString::fromUtf8(pdch->desc), SRD_INITIAL_PIN_SAME_AS_SAMPLE0, decoder, pdch}; channels_.push_back(ch); @@ -597,7 +597,7 @@ void DecodeSignal::update_channel_list() if (!ch_added) { // Create new entry without a mapped signal - data::DecodeChannel ch = {id++, true, nullptr, + data::DecodeChannel ch = {id++, 0, true, nullptr, QString::fromUtf8(pdch->name), QString::fromUtf8(pdch->desc), SRD_INITIAL_PIN_SAME_AS_SAMPLE0, decoder, pdch}; channels_.push_back(ch); @@ -654,8 +654,11 @@ void DecodeSignal::mux_logic_samples(const int64_t start, const int64_t end) vector signal_in_bytepos; vector signal_in_bitpos; + int id = 0; for (data::DecodeChannel &ch : channels_) if (ch.assigned_signal) { + ch.bit_id = id++; + const shared_ptr logic_data = ch.assigned_signal->logic_data(); const shared_ptr segment = logic_data->logic_segments().front(); segments.push_back(segment); diff --git a/pv/data/decodesignal.hpp b/pv/data/decodesignal.hpp index 0e091152..04f3b77f 100644 --- a/pv/data/decodesignal.hpp +++ b/pv/data/decodesignal.hpp @@ -61,7 +61,8 @@ class SignalData; struct DecodeChannel { - uint16_t id; // Also tells which bit within a sample represents this channel + uint16_t id; ///< Global numerical ID for the decode channels in the stack + uint16_t bit_id; ///< Tells which bit within a sample represents this channel const bool is_optional; const pv::data::SignalBase *assigned_signal; const QString name, desc; -- 2.30.2