jerabaul29 / OpenMetBuoy-v2021a

An easy to build, affordable, customizable, open source instrument for oceanographic measurements - with global Iridium coverage
MIT License
37 stars 7 forks source link

deceiving name of fft_overlap variable #72

Open jerabaul29 opened 1 year ago

jerabaul29 commented 1 year ago

https://github.com/jerabaul29/OpenMetBuoy-v2021a/blob/8a1256e43c19c2be496525f64c2e8e073344b8c8/legacy_firmware/firmware/standard_gps_waves_drifter/params.h#L80-L81

vs.

https://github.com/jerabaul29/OpenMetBuoy-v2021a/blob/8a1256e43c19c2be496525f64c2e8e073344b8c8/legacy_firmware/firmware/standard_gps_waves_drifter/params.h#L88-L89

vs.

https://github.com/jerabaul29/OpenMetBuoy-v2021a/blob/8a1256e43c19c2be496525f64c2e8e073344b8c8/legacy_firmware/firmware/standard_gps_waves_drifter/wave_analyzer.cpp#L181-L183

Note that this well corresponds to a 75% overlap as indicated in the comment of fft_overlap definition, but the fft_overlap name is poorly chosen; really it is the fft_nonoverlap, not the fft_overlap that is being defined (the relation between the two being, fft_nonoverlap = fft_length - fft_overlap). So, while the logics are correct / correspond to the 75% overlap comment, the naming of the variables is very poor / misleading.

Many thanks to @gauteh for pointing this :) . This does not affect the correctness of the computations run, so wont fix for now, but keeping this issue open so that it is visible.