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

Cleanup Part 2 #3

Merged
merged 13 commits into from
Jul 19, 2023
Merged

Cleanup Part 2 #3

merged 13 commits into from
Jul 19, 2023

Conversation

drowe67
Copy link
Owner

@drowe67 drowe67 commented Jul 14, 2023

  • spelling errors
  • clang C code formatting of src, unittest, misc (leave stm32 for now, as that's more complicated to test)
  • ctest to maintain clang-format-ing
  • remove Codec 2 450 & 450WB modes (after discussion with Stefan)
  • remove 2020C (after discussion from PLT)

Further work:

  • remove Ethernet data (TBC discussion with Jeroen)
  • remove FreeDV 2400A and B (delay this until decision on Ethernet data, which uses 2400A/B)
  • see if codec2-new still works with freedv-gui, FreeDATA
  • mv codec2 codec2-dev, mv codec2-new codec2
  • ancient headers
  • README with guidelines for what goes in repo, what doesn't, where to find old code, clang formatting

@tmiw
Copy link
Collaborator

tmiw commented Jul 14, 2023

The current branch doesn't build on macOS:

[ 96%] Building C object unittest/CMakeFiles/freedv_700d_comprx.dir/freedv_700d_comprx.c.o
In file included from /Users/mooneer/devel/codec2-new/unittest/freedv_700d_comprx.c:17:
/Users/mooneer/devel/codec2-new/unittest/../src/comp_prim.h:83:10: error: call to undeclared library function 'sqrtf' with type 'float (float)'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  return sqrtf((a.real * a.real) + (a.imag * a.imag));
         ^
/Users/mooneer/devel/codec2-new/unittest/../src/comp_prim.h:83:10: note: include the header <math.h> or explicitly provide a declaration for 'sqrtf'
/Users/mooneer/devel/codec2-new/unittest/../src/comp_prim.h:91:14: error: call to undeclared library function 'cosf' with type 'float (float)'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  res.real = cosf(phi);
             ^
/Users/mooneer/devel/codec2-new/unittest/../src/comp_prim.h:91:14: note: include the header <math.h> or explicitly provide a declaration for 'cosf'
/Users/mooneer/devel/codec2-new/unittest/../src/comp_prim.h:92:14: error: call to undeclared library function 'sinf' with type 'float (float)'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  res.imag = sinf(phi);
             ^
/Users/mooneer/devel/codec2-new/unittest/../src/comp_prim.h:92:14: note: include the header <math.h> or explicitly provide a declaration for 'sinf'
3 errors generated.
make[2]: *** [unittest/CMakeFiles/freedv_700d_comprx.dir/freedv_700d_comprx.c.o] Error 1
make[1]: *** [unittest/CMakeFiles/freedv_700d_comprx.dir/all] Error 2
make: *** [all] Error 2

I'll add math.h to comp_prim.h and see how that goes.

@tmiw
Copy link
Collaborator

tmiw commented Jul 14, 2023

I can get it to compile with #include <math.h> added to the top of comp_prim.h but I don't know if that's the right place for it since it was failing to compile on a ctest file.

Anyway, the current failing ctests:

The following tests FAILED:
	  1 - test_clang_format (Failed)
	 32 - test_OFDM_modem_octave_datac0_mpp_coded (Failed)
	 33 - test_OFDM_modem_datac0_compression (Failed)
	 45 - test_OFDM_modem_datac0_octave_burst (Failed)
	 46 - test_OFDM_modem_datac1_octave (Failed)
	 47 - test_OFDM_modem_datac3_octave (Failed)
	 51 - test_OFDM_modem_datac4_octave (Failed)
	 52 - test_OFDM_modem_datac13_octave (Failed)

Looks like test_clang_format is failing for me since it's called clang-format-mp-XX on my system (where XX is the version number; apparently I have clang 14, 15 and 16 installed thanks to MacPorts).

The others seem to be failing similarly to the following:

    Start 33: test_OFDM_modem_datac0_compression

33: Test command: /bin/sh "-c" "cd /Users/mooneer/devel/codec2-new/unittest;
                            ./check_comp.sh /Users/mooneer/devel/codec2-new /Users/mooneer/devel/codec2-new/build/src"
33: Working Directory: /Users/mooneer/devel/codec2-new/build
33: Environment variables: 
33:  CML_PATH=/Users/mooneer/devel/codec2-new/build/cml
33: Test timeout computed to be: 1500
33: ++ mktemp
33: + octave_log=/var/folders/s9/8rhpy67j2bxfmn80j6q_dfdr0000gn/T/tmp.v6BomiQz
33: ++ mktemp
33: + ch_log=/var/folders/s9/8rhpy67j2bxfmn80j6q_dfdr0000gn/T/tmp.1Jyb59Cm
33: + echo 'warning ('\''off'\'', '\''Octave:data-file-in-path'\'');
33:       ofdm_ldpc_tx('\''test_datac0.raw'\'','\''datac0'\'',1,100,'\''awgn'\'','\''bursts'\'',10,'\''txclip'\''); 
33:       quit'
33: + DISPLAY=
33: + octave-cli -p /Users/mooneer/devel/codec2-new/octave
33: error: fec_encode: function called with too many inputs
33: error: called from
33:     fec_encode
33:     ofdm_ldpc_tx at line 83 column 34

[snipped]

33: + python3 -c 'import sys; sys.exit(0) if abs(( - 6768.93)/) < 0.05 else sys.exit(1)'
33:   File "<string>", line 1
33:     import sys; sys.exit(0) if abs(( - 6768.93)/) < 0.05 else sys.exit(1)
33:                                                 ^
33: SyntaxError: invalid syntax
33: + python3 -c 'import sys; sys.exit(0) if abs(( - 7.68)/) < 0.05 else sys.exit(1)'
33:   File "<string>", line 1
33:     import sys; sys.exit(0) if abs(( - 7.68)/) < 0.05 else sys.exit(1)
33:                                              ^
33: SyntaxError: invalid syntax
1/1 Test #33: test_OFDM_modem_datac0_compression ...***Failed    0.81 sec

@tmiw
Copy link
Collaborator

tmiw commented Jul 14, 2023

I linked this into freedv-gui and did some brief RX testing with the sample .wav files (700C/D/E, 1600, 2020, 2020B) and they all seemed to decode fine. If there are some specific areas that might be more likely to have problems I can try to do some more detailed testing there if desired.

@drowe67
Copy link
Owner Author

drowe67 commented Jul 14, 2023

Thanks for that feedback @tmiw, it does seem like a constant battle to keep the ctests running on macOS, pity we don't have a GitHub actions target.

Yes I also had some problems with clang-format configuration too, I found a version installed by pip under my $(HOME)/.local/bin that was out of date and missing some options, but the Ubuntu version was up to date.

@drowe67
Copy link
Owner Author

drowe67 commented Jul 14, 2023

Re clang-format, I'm using version 14:

david@xps-15:~/tmp/codec2-new/build_linux  dr-cleanup2 $ clang-format --version
Ubuntu clang-format version 14.0.0-1ubuntu1

Not sure how we can make that get automatically selected on macOS 🤔, maybe some cmake code?

@drowe67
Copy link
Owner Author

drowe67 commented Jul 14, 2023

I've pushed some tweaks for the top few issues.

Re the test_OFDM_modem_datac0_compression fails I think it's failing to slurp up oct_rms in unittest/check_comp.sh - take a look a few lines above. The check_comp.sh script can also be run manually outside of the ctest environment - instructions at top. Or perhaps the ofdm_lpdc_tx fix has helped that one too.

@drowe67
Copy link
Owner Author

drowe67 commented Jul 14, 2023

@DJ2LS - when convenient, could you please ask your team to try this branch (dr-cleanup2)? There has been a lot of code reformatting, that changed the order of some include files.

@tmiw
Copy link
Collaborator

tmiw commented Jul 14, 2023

Current macOS failures are as follows:

The following tests FAILED:
	  1 - test_clang_format (Failed)
	 32 - test_OFDM_modem_octave_datac0_mpp_coded (Failed)
	 46 - test_OFDM_modem_datac1_octave (Failed)
	 47 - test_OFDM_modem_datac3_octave (Failed)
	 51 - test_OFDM_modem_datac4_octave (Failed)

Looks like the same fec_encode error for the others:

32: error: fec_encode: function called with too many inputs
32: error: called from
32:     fec_encode
32:     ofdm_ldpc_rx at line 61 column 11
1/1 Test #32: test_OFDM_modem_octave_datac0_mpp_coded ...***Failed  Required regular expression not found. Regex=[Pass
]  1.12 sec
46: error: fec_encode: function called with too many inputs
46: error: called from
46:     fec_encode
46:     ofdm_ldpc_rx at line 61 column 11
1/1 Test #46: test_OFDM_modem_datac1_octave ....***Failed  Required regular expression not found. Regex=[Coded PER: 0.0000 Pckts:     4
]  2.20 sec
47: error: fec_encode: function called with too many inputs
47: error: called from
47:     fec_encode
47:     ofdm_ldpc_rx at line 61 column 11
1/1 Test #47: test_OFDM_modem_datac3_octave ....***Failed  Required regular expression not found. Regex=[Coded PER: 0.0000 Pckts:     5
]  0.74 sec
51: error: fec_encode: function called with too many inputs
51: error: called from
51:     fec_encode
51:     ofdm_ldpc_rx at line 61 column 11
1/1 Test #51: test_OFDM_modem_datac4_octave ....***Failed    0.71 sec

Re: clang-format, apparently I just need to run the following for MacPorts (Homebrew is likely going to be different):

Mooneer6MBP2461:build mooneer$ sudo port select --set clang mp-clang-16

and now I have a /opt/local/bin/clang-format symlink. The failure is different now, though:

1: Test command: /bin/sh "-c" "cd /Users/mooneer/devel/codec2-new;
                            clang-format --dry-run --Werror src/*.c src/*.h unittest/*.c demo/*.c"
1: Working Directory: /Users/mooneer/devel/codec2-new/build
1: Test timeout computed to be: 1500
1: src/freedv_api.c:1417:47: error: code should be clang-formatted [-Wclang-format-violations]
1: struct FSK *freedv_get_fsk(struct freedv *f) {
1:                                               ^
1: src/freedv_api.c:1418:17: error: code should be clang-formatted [-Wclang-format-violations]
1:   return f->fsk;
1:                 ^
1: src/freedv_api.c:1470:53: error: code should be clang-formatted [-Wclang-format-violations]
1: struct CODEC2 *freedv_get_codec2(struct freedv *f) {
1:                                                     ^
1: src/freedv_api.c:1471:20: error: code should be clang-formatted [-Wclang-format-violations]
1:   return f->codec2;
1:                    ^
1: unittest/tfifo.c:29:3: error: code should be clang-formatted [-Wclang-format-violations]
1: //#define USE_MUTEX
1:   ^
1: unittest/tfmfsk.c:30:3: error: code should be clang-formatted [-Wclang-format-violations]
1: //#define MODEMPROBE_ENABLE
1:   ^
1: unittest/tfsk.c:30:3: error: code should be clang-formatted [-Wclang-format-violations]
1: //#define MODEMPROBE_ENABLE
1:   ^
1: unittest/tofdm.c:385:3: error: code should be clang-formatted [-Wclang-format-violations]
1: //#define TESTING_FILE
1:   ^
1/1 Test #1: test_clang_format ................***Failed   12.78 sec

@drowe67
Copy link
Owner Author

drowe67 commented Jul 15, 2023

@tmiw - I've had another attempt at the ctest fix around fec_encode. That's a pretty easy fix BTW (identical to the last one), if you get something similar you might be able to fix it locally:

david@xps-15:~/tmp/codec2-new/octave  dr-cleanup2 $ grep fec_encode *.m
ofdm_ldpc_rx.m:  tx_bits = fec_encode(states, code_param, mode, payload_bits, Ncodecframespermodemframe, Nbitspercodecframe);
ofdm_ldpc_tx.m:  [packet_bits bits_per_packet] = fec_encode(states, code_param, mode, payload_bits);
ofdm_lib.m:function [frame_bits bits_per_frame] = fec_encode(states, code_param, mode, payload_bits)

Not sure about the problems with clang-format, could be a different version, or not picking up the style file codec2/.clang-format. Works OK on Ubuntu 20 & 22, and GitHub actions.

@tmiw
Copy link
Collaborator

tmiw commented Jul 16, 2023

I ran the clang-format test on my Ubuntu 22.04 VM (which worked) and it looks like the "clang-format" package there is from Clang 14. I can get that same test to pass on macOS by switching the active Clang version in MacPorts to 14 as well (I had version 16 active before). Not sure if that helps at all.

Anyway, all good now (at least on x86_64):

138/138 Test #138: test_demo_datac0c1 .............................   Passed    3.03 sec

100% tests passed, 0 tests failed out of 138

Total Test time (real) = 416.81 sec

@drowe67
Copy link
Owner Author

drowe67 commented Jul 19, 2023

@tmiw @DJ2LS I'm going to merge this, then do the cutover to make this the main codec2 repo:

rename codec2 -> codec2-dev
rename codec2-new -> codec2

Fingers crossed 🤞 - and we can always swap back if I've busted something.

Still some clean up items to go - I haven't been able to make email contact with Jeroen re the Ethernet data stuff, which is connected to the PLT level decision to remove FreeDV 2400A/B.

@drowe67 drowe67 merged commit 06d4c11 into main Jul 19, 2023
@tmiw
Copy link
Collaborator

tmiw commented Jul 24, 2023

@tmiw @DJ2LS I'm going to merge this, then do the cutover to make this the main codec2 repo:

rename codec2 -> codec2-dev
rename codec2-new -> codec2

Fingers crossed 🤞 - and we can always swap back if I've busted something.

Still some clean up items to go - I haven't been able to make email contact with Jeroen re the Ethernet data stuff, which is connected to the PLT level decision to remove FreeDV 2400A/B.

Seems that the GitHub actions for freedv-gui are breaking now since there aren't any releases on the new repo. For example: https://github.com/drowe67/freedv-gui/actions/runs/5642884170/job/15283637908?pr=481

@l29ah
Copy link

l29ah commented Oct 6, 2023

Why did you remove 450?

@IhorNehrutsa
Copy link

Why did you remove 450?

The same question.

remove Codec 2 450 & 450WB modes (after discussion with Stefan)

Is the discussion with Stefan in a public area?

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.

4 participants