search

veeresht / CommPy

Digital Communication with Python
http://veeresht.github.com/CommPy
BSD 3-Clause "New" or "Revised" License
551 stars 179 forks source link

Fix CAZAC sequence #91

Closed londumas closed 3 years ago

londumas commented 3 years ago

Fix issue https://github.com/veeresht/CommPy/issues/70. The CAZAC sequence was improperly coded, leading to non-zero cross-correlation terms when seq_length was an even number. This is fixed using the formula from https://en.wikipedia.org/wiki/Zadoff%E2%80%93Chu_sequence, thanks to the cf term. Furthermore, additional checks are done to see if inputs are valid.

londumas commented 3 years ago

Tests failed, but I bet it is because of runtime, on my computer there were no issues:

<me>/CommPy/commpy/tests$ python test_sequences.py 
.
----------------------------------------------------------------------
Ran 1 test in 0.001s

OK
BastienTr commented 3 years ago

It make sense when looking at Travis logs. I pushed an empty commit on the master branch to confirm this assumption.

coveralls commented 3 years ago

Coverage Status

Coverage increased (+0.4%) to 82.799% when pulling ac51ec65a6767c903ab0f2bbae420054e69d2c08 on londumas:fix-cazac-sequence into bd47de10449efc81c286e229ca80cc484d6f1e08 on veeresht:master.

londumas commented 3 years ago

@BastienTr, the travis checks are ok, but now it says coverage is not ok. Do you want me to do anything?

BastienTr commented 3 years ago

Coveralls is not happy with the coverage decrease. It would be good to add a test in tests/test_sequences.py that checks the value from a known sequence and check that the assertions are properly raised.

Besides, I recommend switching from assert to error raising. As a reference you can have a look to the modulation code, doc and test.

Anyway, thanks for the contribution :+1:

londumas commented 3 years ago

@BastienTr This commit should fix these different issues and recommendations. I also corrected "test_pnsequence" where two asserts where in a with loop, removing the test of the second test.