Feat/acoustics#42
Conversation
1 FDCAN 1 SPI master 3 SPI slaves 1 Usart
allow cli make clean all build project used to generate compile_json
remove all incompatible *f16.c from CMSIS.DSP remove all incompatible ARM cortex A (neon) from CMSIS_DSP remove all source files that include all source files in the same directory
First iteration, made completely by chatgtp
…-auv-embedded into feat/acoustics
bruh
|
Idk man, cyp ville ha deg med |
forrisdahl
left a comment
forrisdahl
left a comment
There was a problem hiding this comment.
I do not know why, but the GitHub browser is insanely slow and difficult to mark where I want to comment, so I will make one larger comment instead.
My general impression of main is that it is big. It has a lot of functions, and it is not immediately clear which are important/used and which are not. I suggested splitting the debug or printf functions into another file. I understand that this might be challenging due to variable scope. The most important thing to see in the main.c file is the main function. I would like the main function to be as early as possible, with other functions defined below it but declared in the header or before main.
I think the code inside the while loop could also be split into functions. It does not have to be many functions, but as it is now, we end up with a lot of indentation. Function names could also make it easier to understand what each step is trying to achieve.
It is a bit random where there are comments and where there are not. For the most important functions, you should have Doxygen-style comments to explain what they do and what they expect.
//NOT TESTED AT TIME OF COMMIT// cleaned and modularized everything all that remains should be to implement dumping functionality and test Features added: CAN interface Proper state machine Stale Data detection
Left to implement:
Dump commands
Making it not error due to stale data every 10 seconds
|
Idk what i'm doing. |
…SPI interface would fail. Fixed it. Acoustics is now megagood. Dump still not implemented
|
Fixed the erroring out thingy now. |
| while(((idxs[0] < dead_space) || (idxs[0] > (WORKSPACE_LEN-(WORKSPACE_OFFSET-1)*BLOCK_LEN))) && max_retries--){ | ||
| float32_t threshold = dsp_min_max_lerp(processing_workspace[0], WORKSPACE_LEN, linear_threshold, n_upper_average, dead_space); | ||
| idxs[0] = dsp_rl_under_threshold_search(processing_workspace[0],WORKSPACE_LEN, threshold , PROCESSING_PATIENCE); | ||
| linear_threshold *= 2; | ||
| } | ||
| times_of_arrival[0] = (float32_t)idxs[0]; | ||
|
|
||
| for(int i = 1; i < N_HYDROPHONES; i++){ | ||
| float32_t linear_threshold = lerp_threshold; | ||
| uint8_t max_retries = 6; | ||
| uint32_t dead_space = n_lower_average; | ||
| idxs[i] = 0; | ||
| while(((utils_abs_int32(idxs[0] - idxs[i])) > max_idx_difference) && max_retries--){ | ||
| float32_t threshold = dsp_min_max_lerp(processing_workspace[i], WORKSPACE_LEN, linear_threshold, n_upper_average, dead_space); | ||
| idxs[i] = dsp_rl_under_threshold_search(processing_workspace[i],WORKSPACE_LEN, threshold , PROCESSING_PATIENCE); | ||
| linear_threshold *= 2; | ||
| } | ||
| times_of_arrival[i] = (float32_t)idxs[i]; | ||
| } | ||
|
|
||
| valid_buffers = 0; |
There was a problem hiding this comment.
Some of these conditions are quite cryptic, not immediately obvious what they are checking. You could have a static inline function to have a human readable name for the conditions.
You probably should split the first part of the function, (the searching?) to its own function. Currently function does a lot of things. Could have one function that finds the TDOA and validates the data and one that solves the position
|
Aight now there are surely no other problems left |
used outside of acoustics.c
|
This looks a lot better, much easier to understand the acoustics processing. What remains now is to clean up your branch. First of all I see that there are telemetry files I accidentally pushed on the branch that needs to be removed. I think the old version of the firmware can be removed to save space. Both the old version and the new version includes CMSIS library which adds a lot more files to your branch than needs to be. In addition since I do not think we have done any changes cmsis library we should look for a way to avoid having it in our repo. |
3 participants
It works, but needs a bit of polish