Closed 0xMihir closed 2 years ago
I am not exactly sure what you are referring to, so I am guessing it's about an_x and an_y arrays, correct? Indeed, some calculations can be performed separately for IR and RED channels using one array, but what about the rf_Pcorrelation() call? It needs both at the same time.
Rather than creating the an_x and an_y arrays, I modified the code to modify the arrays passed in the function's parameter. I realized that saying single float buffer was unclear.
Thank you for good contribution, but I cannot accept the pull request as is because it seems incomplete - it does not contain matching changes to RD117_ARDUINO.ino. It still contains: uint32_t aun_ir_buffer[BUFFER_SIZE]; which is passed to your version of rf_heart_rate_and_oxygen_saturation() as float pointer. Perhaps there is an implicit conversion from pointer to uint32_t but I would not bet that all compilers do it.
Yep, I'll change it to float
.
Hi 0xMihir, I had to revert your pull request, sorry, since it would break the code in this repository. In fact, other functions in other parts of the code still use uint32_t incompatible with float. For example, maxim_max30102_read_fifo() in max30102.cpp or the original MAXIM algorithm in algorithm.cpp.
The first part of the
rf_heart_rate_and_oxygen_saturation
code allocates two float arrays to perform calculations on. The default buffer size of 100 results in 800 bytes (2 arrays 4 bytes 100 elements) of memory usage, making many libraries that require some dynamic memory impossible to use in conjunction with this one.I know that it's the best convention to leave parameters unmodified, but when working on underpowered microcontrollers like the Arduino Uno, performing all calculations on a single float buffer makes this library use a lot less memory. I have already modified the code to work this way, but I wanted to hear your thoughts before opening a PR.