Fix frame decoding #224

ebroecker · 2018-10-26T21:57:14Z

this is NOT ready for merge, but in a PR the could be better done.

this fixes wrong decoding frames (#119) BUT this is only proof of concept, further cleanup is needed (encoding, multiplex support, dismiss old code ...)

ebroecker · 2018-10-28T21:48:36Z

@altendky
your unpack code worked out out the box ...

Now I tried to use your pack code also, but stumbled over some magic code:
https://github.com/altendky/st/blob/c75f436f503adcb4708131a65b059c8062484ae5/epyqlib/canneo.py#L136-L147
I am not sure what happens here, maybe you could give me a hint?
Thanks in advance

altendky · 2018-10-29T00:14:17Z

Let's start with a vague overview... I'm just rereading the code since I really don't remember this stuff.

Keep in mind that I wanted to allow for overlapped signals and mixed big and little endian as well. Unpacking overlapped signals... Whatever. A single bit from the bus might end up in multiple signals, no conflict. Packing though, you may have to prioritize one over another, or report a conflict (this code prioritizes I think which is probably a bad choice).

I think the first two lines take the bits calculated from little endian signals and 'turn them around' to align with the big endian bits.

bitstring = iterates over the little and big bits together. The next() picks the first non-None from the choices of the little endian bit, the big endian bit, and the default '0'.

The return line groups the bits into groups of 8, joins each group to an eight character string, creates an integer from the base-2 string, and creates a bytes object from that iterable of integers.

Maybe?

add dbc-files in test folder to egg

codecov-io · 2018-10-30T10:53:56Z

Codecov Report

Merging #224 into development will increase coverage by 5.19%.
The diff coverage is 90.07%.

@@               Coverage Diff               @@
##           development     #224      +/-   ##
===============================================
+ Coverage        17.48%   22.68%   +5.19%     
===============================================
  Files               25       27       +2     
  Lines             5960     6168     +208     
  Branches          1604     1609       +5     
===============================================
+ Hits              1042     1399     +357     
+ Misses            4852     4660     -192     
- Partials            66      109      +43

Impacted Files	Coverage Δ
src/canmatrix/tests/test_frame_decoding.py	`100% <100%> (ø)`
src/canmatrix/tests/test_canmatrix.py	`100% <100%> (ø)`	⬆️
src/canmatrix/tests/test_frame_encoding.py	`84.31% <84.31%> (ø)`
src/canmatrix/canmatrix.py	`54.54% <89.28%> (+10.22%)`	⬆️
src/canmatrix/sym.py	`36.72% <0%> (+3.47%)`	⬆️
src/canmatrix/formats.py	`49.45% <0%> (+9.89%)`	⬆️
src/canmatrix/dbc.py	`15.33% <0%> (+12.66%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0cc1073...3d5f6f9. Read the comment docs.

add float decoding tests add mux decoding tests fix py2 compatibility

altendky

I would break the test down into five separate tests. At least three: integers, floats, and multiplexed.

src/canmatrix/dbc.py

src/canmatrix/tests/test_frame_decoding.py

remove not needed python version checking

Encoding tests still failing. guess: something with sign seems to be wrong TODO: check if test data is correct

altendky · 2018-10-31T23:59:09Z

I'm not sure at the moment but we should be able to monitor for unrun tests (like would happen with matching names) via coverage of the tests.

ebroecker · 2018-11-01T23:21:35Z

src/canmatrix/canmatrix.py

@@ -400,7 +400,7 @@ def pack_bitstring(self, length, is_float, value, signed):
                               signed=signed)
            b = '{:0{}b}'.format(int.from_bytes(b, byteorder='big'),
                                 length)
-            bitstring = b[:length]


@altendky
seems I found a bug. At least now my tests seem to produce correct data.
This was your code, maybe you should have a look to your code also...

Huh... I wonder how I haven't had issues. It seems to show up for signed values (I forget big endian or little or...). Anyways, thanks for pointing that out. As a double check I made up #235 to recreate it 'in my world' of .sym files.

ebroecker · 2018-11-02T00:16:06Z

@altendky
this PR is getting close to done - in my eyes.
One open thing is to kick out bitstruct of the requirements, but the code should work now as expected...

Could you do another review?

Thanks

altendky

Alrighty... I'll try to finish up the rest tomorrow. Thanks for all this, of course.

src/canmatrix/canmatrix.py

altendky

alrighty... i'll probably have to either take a pass three later or reserve the right to think that followup changes might be good. Probably better to do the latter, if at all. i don't like dragging out pr's forever...

src/canmatrix/canmatrix.py

src/canmatrix/tests/test_frame_decoding.py

Co-Authored-By: ebroecker <eduard.broecker@gmail.com>

* Add .sym based test * fixup for py2

…trix into fixFrameDecoding

ebroecker added 2 commits October 25, 2018 22:32

add tests and dbc for frame decoding

3e70b14

add unpack code from altendky,

01e727a

this fixes wrong decoding frames (#119) BUT this is only proof of concept, further cleanup is needed (encoding, multiplex support, dismiss old code ...)

altendky changed the title ~~Fix frame decoding~~ [WIP] Fix frame decoding Oct 26, 2018

altendky mentioned this pull request Oct 29, 2018

findNotUsedBits() reports the signals index+1, not the index #227

Closed

ebroecker added 2 commits October 29, 2018 23:05

adoption of altendky's pack/encode code

9328417

Update setup.py

6df2386

add dbc-files in test folder to egg

ebroecker added 2 commits October 30, 2018 23:25

Merge branch 'development' into fixFrameDecoding

94bbd9b

add float decoding tests add mux decoding tests fix py2 compatibility

fix pypy compatibility

baaccb6

altendky requested changes Oct 31, 2018

View reviewed changes

src/canmatrix/dbc.py Outdated Show resolved Hide resolved

src/canmatrix/dbc.py Outdated Show resolved Hide resolved

src/canmatrix/tests/test_frame_decoding.py Outdated Show resolved Hide resolved

ebroecker added 5 commits October 31, 2018 22:38

Merge branch 'development' into fixFrameDecoding

a994c38

split frame decoding tests into multiple tests

3fe50b5

remove not needed python version checking

Add encoding tests.

b9ddc1f

Encoding tests still failing. guess: something with sign seems to be wrong TODO: check if test data is correct

add missing and failing encoding test

656698d

fix copy paste failure in test

2a0d400

ebroecker added 2 commits November 1, 2018 23:39

fix frame encoding (#119)

69bead4

add docstrings for frame encoding/decoding (#119)

6ffdcd7

ebroecker commented Nov 1, 2018

View reviewed changes

make python2 happy (#224)

a187780

ebroecker changed the title ~~[WIP] Fix frame decoding~~ Fix frame decoding Nov 2, 2018

cleanups (less version dependencies )

c7567ec

altendky requested changes Nov 3, 2018

View reviewed changes

altendky reviewed Nov 3, 2018

View reviewed changes

src/canmatrix/canmatrix.py Show resolved Hide resolved

src/canmatrix/canmatrix.py Outdated Show resolved Hide resolved

src/canmatrix/tests/test_frame_decoding.py Show resolved Hide resolved

altendky and others added 3 commits November 3, 2018 17:47

Update src/canmatrix/canmatrix.py

4f17ced

Co-Authored-By: ebroecker <eduard.broecker@gmail.com>

Update src/canmatrix/canmatrix.py

169c091

Co-Authored-By: ebroecker <eduard.broecker@gmail.com>

[WIP] Add .sym based encoding test (#235)

2b90d1e

* Add .sym based test * fixup for py2

ebroecker added 3 commits November 3, 2018 21:57

further work on pr #224

0d3e9d3

Merge branch 'fixFrameDecoding' of https://github.com/ebroecker/canma…

bf523ca

…trix into fixFrameDecoding

work on altendkys review finding for #224

3d5f6f9

ebroecker merged commit eb6c2ed into development Nov 4, 2018

ebroecker mentioned this pull request Nov 4, 2018

Decode/Encode Messages - Little Endian/Big Endian #119

Closed

ebroecker deleted the fixFrameDecoding branch November 4, 2018 21:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix frame decoding #224

Fix frame decoding #224

ebroecker commented Oct 26, 2018

ebroecker commented Oct 28, 2018

altendky commented Oct 29, 2018

codecov-io commented Oct 30, 2018 •

edited

Loading

altendky left a comment

altendky commented Oct 31, 2018

ebroecker Nov 1, 2018

altendky Nov 2, 2018

ebroecker commented Nov 2, 2018

altendky left a comment

altendky left a comment

Fix frame decoding #224

Fix frame decoding #224

Conversation

ebroecker commented Oct 26, 2018

ebroecker commented Oct 28, 2018

altendky commented Oct 29, 2018

codecov-io commented Oct 30, 2018 • edited Loading

Codecov Report

altendky left a comment

Choose a reason for hiding this comment

altendky commented Oct 31, 2018

ebroecker Nov 1, 2018

Choose a reason for hiding this comment

altendky Nov 2, 2018

Choose a reason for hiding this comment

ebroecker commented Nov 2, 2018

altendky left a comment

Choose a reason for hiding this comment

altendky left a comment

Choose a reason for hiding this comment

codecov-io commented Oct 30, 2018 •

edited

Loading