fix the output formatting of table mode

[rohoog]

See issue #29

Sorry I'm not familiar with this more pythonic .format method; I used the trusty old printf style 'interpolation' instead.

[rohoog]

Looks like the failures are due to upper/lower case hex. This should not be an issue, but I will change %x to %X to get the same casing as before.

[Nicoretti]

Hi @rohoog,

thanks for creating a PR to address #29, really appreciate it.
A question from my side, could you add a regression test for the issue at hand?
(So we can be sure the behavior will stay correct in the future)

You can have a look at cli_test.py to get an idea how such a test could look like.

Regarding the python format string vs printf style, I rather would like to keep the format string one, but I will be happy to help you out there if you want me to. In case you want to dig into it by yourself first these links may help:

• format string syntax
• format spec

At first glance I think the alignment within the format string is the issue <
(We may just need to remove or replace it with the appropriate alignment)

best
Nico

[rohoog]

I am not such a python guru as you might think...
The way to test such things is to use the table exporting, put it in a file (or pipe) and read it back in as the contents of the _lookup of the TableBasedCrcRegister and compare the result of CRC remainder from the built-in generated table (which works fine) and the remainder using the table read back from the file. This way you can easily cover all cases (although one case is enough to detect the bug).

[Nicoretti]

@rohoog
I don't think covering all cases is needed, but having the use case which exposed the bug would be nice to have.
I think this can be done similarly like in the test by just adjusting the expected_output and the argv which
have been used.

I am not such a python guru as you might think...

No problem I will guide you if you are interested in getting your PR integrated.
If you feel overwhelmed or just want to get the issue out of the way I recommend you fix it in your fork and use that one for
now. I try to address the issue as soon as I can make some time for it.

[rohoog]

Could it even be that the expected output for the crc8 poly 0x1D is actually wrong? So that just fixing that one will 'test' it properly? I see no values starting with 0x0 (except for 0x00), while there should definitely be some.
I mean just inserting the (now correctly) outputted table for that polynomial is sufficient. I think it is a bit /wrong/ to test like that... but that is another discussion.

[rohoog]

OK done that as well.

[rohoog]

Implementation result driven testing instead of test driven development.... 😕

[Nicoretti]

Implementation result driven testing instead of test driven development.... confused

I need to hit the bed now. As soon as I can I will validate that outputted tables are correct.
If we validate that the output is correct, then add it and keep them as test I rather would say it's a
"snapshot" based "integration" test. Similar to what cram and prysk are doing.

If we only would add without checking I agree it would not really a "test".

[rohoog]

Hurray! Success!

[Nicoretti]

I have checked the table, looks like the one you added is correct. Apparently this test was setup incorrectly.