Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add NRZI encoding decoding #12

Merged
merged 6 commits into from
Mar 18, 2024
Merged

add NRZI encoding decoding #12

merged 6 commits into from
Mar 18, 2024

Conversation

ErikBuer
Copy link
Contributor

Hi professor Gerzaguet, hope everything is well.

I have added NRZI encoding decoding with some explanatory tests.
First, I thought the functions were suitable for the file bitMapping, as it is a mapping to the NRZI format.
But the file is in the documentation under something QAM-related, so I made a separate file for them.

I have also added some quality-of-life enhancements for building docs locally.
This will make it faster to build docs locally, and should not affect the docs build in CI. I have done similarly in my package radioPropagation.

In the documentation I see you have used the term "bit sequence" and "byte sequence". In other sources, I see people using the terms packed bits and unpacked bits, bit array and byte array.
I feel we should make a small write-up (example in the docs) to explain/visualize the terms and what they mean. Using the various terms will hopefully help the library appear in searches, helping others on their journey.
This must be in a separate PR, but I thought I'd mention it..
I have adopted the term bit sequence, to stay consistent in this library.

Signed-off-by: Erik Buer <buer94@gmail.com>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for this !

readme.md Show resolved Hide resolved
@RGerzaguet
Copy link
Member

Many thanks Eric for this very complete and useful add of NRZI !
Few remarks (I have updated the code except the last point)

  • While the API is great (and consistent with QAM modulation for instance) I think that we should also add bang methods (!) with pre-allocation
  • The code may be a little optimized in terms of pre-allocation to be able to run at full speed. While this is not a key element for classic simulations, it may help when it comes to high performance needs. As we now the size of the output this is not that tricky to code I believe.
  • Last point let's add some unit tests :)
    On your side, may you add a test_NRZI.jl file in test/ with something like
# ---------------------------------------------------- 
# --- Import modules  
# ---------------------------------------------------- 
using DigitalComm 
using Test
# ---------------------------------------------------- 
# --- Tests 
# ---------------------------------------------------- 
# Note --> The mapping system is described in bitMapping.jl
println("Tests for symbol mapper with NRZI sequences");

@testset  "NRZI" begin 
	# Create a bit squence (already tested) 
	nbBits	= 2 * 2048;
	bitSeq  = genBitSequence(nbBits);
	# Pass trough the function 
	buff	= zeros(Complex{Float64},nbBits)
	# Call 
	encodeNRZI!(buff,bitSeq); 
	buff2	= encodeNRZI(bitSeq);
	@test all( buff .== buff2)
        # Ensure Tx // Rx is Ok 
        @test all(bitSeq .== decodeNRZI(encodeNRZI(bitSeq)))
        @test all(bitSeq .== decodeNRZI(encodeNRZI(bitSeq,:high),:high))
	# Some manual check for both transitions
	buff  = [0x01;0x00;0x00;0x01;0x00;0x00;0x00;0x01 ];
        @test all( encodeNRZI(buff,:low) .== [0;1;0;0;1;0;1;1])  # Transitions on 0 
        @test all( encodeNRZI(buff,:high) .== [1;1;1;0;0;0;0;1]) # Transitions on 1
end

and the associated include in the core test suite (runtests.jl) line 19

# NRZI Mapping 
include("test_NRZI.jl");

Thank you very much for keeping DigitalComm such active and for contributing, I appreciate it a lot.

@RGerzaguet
Copy link
Member

Re @ErikBuer
If this is Ok for you, I will merge this PR and create a new version for DigitalComm.
If you want to update the code further, do not hesitate.
Thanks again !

@RGerzaguet RGerzaguet merged commit 6a53033 into JuliaTelecom:master Mar 18, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants