jopohl / urh

Universal Radio Hacker: Investigate Wireless Protocols Like A Boss
GNU General Public License v3.0
10.99k stars 874 forks source link

Analysis View: display of "selected values" below grid is wrong when in Hex/ASCII mode #939

Closed nospam2000 closed 2 years ago

nospam2000 commented 2 years ago
Expected Behavior

The displayed values of the "selected columns" for "Bit:", "Hex:" and "Decimal:" below the grid should be correct.

Actual Behavior

The start and end offsets of the bit-range are calculated incorrectly. For the Hex value that means the offset is shifted by 3 and the ASCII offset is shifted by 7.

Steps To Reproduce
  1. Go to 'Analysis' tab
  2. Change "view data as" to "Hex"
  3. Click on any column of the signal and compare the value from the cell with the values of "selected value" below the grid
  4. The offset of the "selected" values of "Bit:", "Hex:" and "Decimal:" are all shifted left by 3 bits, that means they contain 3 bits from the next cell
Screenshots
Screenshot1: shows error

The selected cell contains the value '5' but below the grid the value 'f' is displayed:

shown_value = (0x5 << 3 | 0xe >> 1) & 0x0f = 0xf

Actually the bit offsets from 133 to 136 are used instead of 130 to 133.

Screenshot1
Screenshot2: shows correct behaviour in "Bit" mode

In "Bit" mode the display is correct:

Screenshot2
Platform Specifications
OS: macOS 11.6.3 with macports
XCode: 13.2.1 (including CLI tools)
URH version: urh-2.9.2.tar.gz
Python version: 3.10 installed with macports
pyqt5: py310-pyqt5 @5.15.6
bladeRF: @20211028-5a146b2a installed with macports
urh installed via: sudo -H BLADERF_API_VERSION=2.41 SDRPLAY_API_VERSION=2.13 pip-3.10 install urh
Possible fix

The calculation of 'start' and 'end' in CompareFrameController.py:on_table_selection_timer_timeout() is incorrect because the range of the column numbers starts from '1' but the multiplication needs to be done on a base of zero and then the '1' added afterwards.

--- CompareFrameController.py.orig  2022-02-20 07:50:34.000000000 +0100
+++ /opt/local/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/urh/controller/CompareFrameController.py  2022-02-20 08:28:39.000000000 +0100
@@ -1359,7 +1359,7 @@
         }

         f = 4 if cur_view == 1 else 8 if cur_view == 2 else 1
-        start, end = start * f, end * f
+        start, end = (start - 1) * f + 1, (end - 1) * f + 1

         bits = message.decoded_bits_str[start-message.alignment_offset:end-message.alignment_offset]
         hexs = "".join(("{0:x}".format(int(bits[i:i + 4], 2)) for i in range(0, len(bits), 4)))
andynoack commented 2 years ago

I cannot confirm that in general. The "selected values" display depends on label borders. When they are not byte or nibble aligned there can be a mismatch actually. Your fix seems legit but I don't know whether it would have impact on any other (internal) behavior right now. @jopohl what do you think?

nospam2000 commented 2 years ago

@andynoack

The "selected values" display depends on label borders.

That was not clear to me, but it makes sense and explains the behaviour.

When they are not byte or nibble aligned there can be a mismatch actually.

In my case a label starts at bit column 102, nibble alignment would be at 101.

Your fix seems legit but I don't know whether it would have impact on any other (internal) behavior right now.

When looking at the bits which don't have a label they are decoded wrong with my patch, so forget about my patch.

The "selected values" show the real value at the offset from the column regardless of any labels. The values in the grid use the labels and may shift the bit-stream for a label aligned interpretation.

Here an example where I disabled the labels for the first two packets. The raw data is the same for packets 1 to 7, but the matrix display gets adjusted according to the label:

Screenshot3

The marked cells "C" + "E" are displayed as "72" like the values from packet 1 and 2. This is ok once the behaviour of labels is clear.