Atomic HRS reads
- Combines the reading of all HRS3300 registers into one I2C read so data is not partial
- Downsizes both HRS and ALS to 16bit as the sensor does not generate larger than 16bit values in its current configuration
- Increasing the resolution by 1 bit doubles the sensor acquisition time, since we are already at 10Hz we are never going to use a higher resolution
- The PPG algorithm buffers for ALS/HRS are already 16bit anyway
- Removes functions for setting gain / drive that are unused throughout the codebase
Build size and comparison to main:
| Section | Size | Difference |
|---|---|---|
| text | 374216B | -32B |
| data | 948B | 0B |
| bss | 63480B | 0B |
Thought I may as well calculate the required buffer length too. Fewer magic numbers
Well seems to be back :see_no_evil:
found an issue that maybe is related: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116634
[build] /home/nero/repos/pinetime/InfiniSim/InfiniTime/src/drivers/Hrs3300.cpp: In member function ‘Pinetime::Drivers::Hrs3300::PackedHrsAls Pinetime::Drivers::Hrs3300::ReadHrsAls()’:
[build] /home/nero/repos/pinetime/InfiniSim/InfiniTime/src/drivers/Hrs3300.cpp:74:83: error: no matching function for call to ‘begin(const Pinetime::Drivers::Hrs3300::Registers [6])’
[build] 74 | constexpr uint8_t baseOffset = static_cast<uint8_t>(*std::min_element(std::begin(dataRegisters), std::end(dataRegisters)));
[build] | ~~~~~~~~~~^~~~~~~~~~~~~~~
[build] In file included from /usr/include/c++/14.2.1/bits/algorithmfwd.h:39,
[build] from /usr/include/c++/14.2.1/bits/stl_algo.h:59,
[build] from /usr/include/c++/14.2.1/algorithm:61,
[build] from /home/nero/repos/pinetime/InfiniSim/InfiniTime/src/drivers/Hrs3300.cpp:8:
[build] /usr/include/c++/14.2.1/initializer_list:88:5: note: candidate: ‘template<class _Tp> constexpr const _Tp* std::begin(initializer_list<_Tp>)’
[build] 88 | begin(initializer_list<_Tp> __ils) noexcept
[build] | ^~~~~
[build] /usr/include/c++/14.2.1/initializer_list:88:5: note: template argument deduction/substitution failed:
[build] /home/nero/repos/pinetime/InfiniSim/InfiniTime/src/drivers/Hrs3300.cpp:74:83: note: mismatched types ‘std::initializer_list<_Tp>’ and ‘const Pinetime::Drivers::Hrs3300::Registers*’
[build] 74 | constexpr uint8_t baseOffset = static_cast<uint8_t>(*std::min_element(std::begin(dataRegisters), std::end(dataRegisters)));
[build] | ~~~~~~~~~~^~~~~~~~~~~~~~~
[build] /home/nero/repos/pinetime/InfiniSim/InfiniTime/src/drivers/Hrs3300.cpp:74:108: error: no matching function for call to ‘end(const Pinetime::Drivers::Hrs3300::Registers [6])’
[build] 74 | constexpr uint8_t baseOffset = static_cast<uint8_t>(*std::min_element(std::begin(dataRegisters), std::end(dataRegisters)));
[build] | ~~~~~~~~^~~~~~~~~~~~~~~
[build] /usr/include/c++/14.2.1/initializer_list:99:5: note: candidate: ‘template<class _Tp> constexpr const _Tp* std::end(initializer_list<_Tp>)’
[build] 99 | end(initializer_list<_Tp> __ils) noexcept
[build] | ^~~
[build] /usr/include/c++/14.2.1/initializer_list:99:5: note: template argument deduction/substitution failed:
[build] /home/nero/repos/pinetime/InfiniSim/InfiniTime/src/drivers/Hrs3300.cpp:74:108: note: mismatched types ‘std::initializer_list<_Tp>’ and ‘const Pinetime::Drivers::Hrs3300::Registers*’
[build] 74 | constexpr uint8_t baseOffset = static_cast<uint8_t>(*std::min_element(std::begin(dataRegisters), std::end(dataRegisters)));
[build] | ~~~~~~~~^~~~~~~~~~~~~~~
[build] /home/nero/repos/pinetime/InfiniSim/InfiniTime/src/drivers/Hrs3300.cpp:77:79: error: no matching function for call to ‘begin(const Pinetime::Drivers::Hrs3300::Registers [6])’
[build] 77 | constexpr uint8_t length = static_cast<uint8_t>(*std::max_element(std::begin(dataRegisters), std::end(dataRegisters))) - baseOffset + 1;
[build] | ~~~~~~~~~~^~~~~~~~~~~~~~~
[build] /usr/include/c++/14.2.1/initializer_list:88:5: note: candidate: ‘template<class _Tp> constexpr const _Tp* std::begin(initializer_list<_Tp>)’
[build] 88 | begin(initializer_list<_Tp> __ils) noexcept
[build] | ^~~~~
[build] /usr/include/c++/14.2.1/initializer_list:88:5: note: template argument deduction/substitution failed:
[build] /home/nero/repos/pinetime/InfiniSim/InfiniTime/src/drivers/Hrs3300.cpp:77:79: note: mismatched types ‘std::initializer_list<_Tp>’ and ‘const Pinetime::Drivers::Hrs3300::Registers*’
[build] 77 | constexpr uint8_t length = static_cast<uint8_t>(*std::max_element(std::begin(dataRegisters), std::end(dataRegisters))) - baseOffset + 1;
[build] | ~~~~~~~~~~^~~~~~~~~~~~~~~
[build] /home/nero/repos/pinetime/InfiniSim/InfiniTime/src/drivers/Hrs3300.cpp:77:104: error: no matching function for call to ‘end(const Pinetime::Drivers::Hrs3300::Registers [6])’
[build] 77 | constexpr uint8_t length = static_cast<uint8_t>(*std::max_element(std::begin(dataRegisters), std::end(dataRegisters))) - baseOffset + 1;
[build] | ~~~~~~~~^~~~~~~~~~~~~~~
[build] /usr/include/c++/14.2.1/initializer_list:99:5: note: candidate: ‘template<class _Tp> constexpr const _Tp* std::end(initializer_list<_Tp>)’
[build] 99 | end(initializer_list<_Tp> __ils) noexcept
[build] | ^~~
[build] /usr/include/c++/14.2.1/initializer_list:99:5: note: template argument deduction/substitution failed:
[build] /home/nero/repos/pinetime/InfiniSim/InfiniTime/src/drivers/Hrs3300.cpp:77:104: note: mismatched types ‘std::initializer_list<_Tp>’ and ‘const Pinetime::Drivers::Hrs3300::Registers*’
[build] 77 | constexpr uint8_t length = static_cast<uint8_t>(*std::max_element(std::begin(dataRegisters), std::end(dataRegisters))) - baseOffset + 1;
[build] | ~~~~~~~~^~~~~~~~~~~~~~~
[build] /home/nero/repos/pinetime/InfiniSim/InfiniTime/src/drivers/Hrs3300.cpp:80:15: error: size of array ‘buf’ is not an integral constant-expression
[build] 80 | uint8_t buf[length];
[build] | ^~~~~~
and I get rid of it including <vector> after <algorithm>??? something is definitely going on with gcc 14.2.1 :see_no_evil:
Strange that it's intermittent. If adding some headers fixes it I suppose it doesn't hurt to?
after more investigating I can say, not a compiler bug. GCC 14 is now more correct and somewhere cleaned up their stlib headers.
We use std::begin, but we don't include any headers that define std::begin
List of headers that define std::begin std::end (from: https://en.cppreference.com/w/cpp/iterator/begin )
Defined in header [<array>](https://en.cppreference.com/w/cpp/header/array)
Defined in header [<deque>](https://en.cppreference.com/w/cpp/header/deque)
Defined in header [<flat_map>](https://en.cppreference.com/w/cpp/header/flat_map)
Defined in header [<flat_set>](https://en.cppreference.com/w/cpp/header/flat_set)
Defined in header [<forward_list>](https://en.cppreference.com/w/cpp/header/forward_list)
Defined in header [<inplace_vector>](https://en.cppreference.com/w/cpp/header/inplace_vector)
Defined in header [<iterator>](https://en.cppreference.com/w/cpp/header/iterator)
Defined in header [<list>](https://en.cppreference.com/w/cpp/header/list)
Defined in header [<map>](https://en.cppreference.com/w/cpp/header/map)
Defined in header [<regex>](https://en.cppreference.com/w/cpp/header/regex)
Defined in header [<set>](https://en.cppreference.com/w/cpp/header/set)
Defined in header [<span>](https://en.cppreference.com/w/cpp/header/span)
Defined in header [<string>](https://en.cppreference.com/w/cpp/header/string)
Defined in header [<string_view>](https://en.cppreference.com/w/cpp/header/string_view)
Defined in header [<unordered_map>](https://en.cppreference.com/w/cpp/header/unordered_map)
Defined in header [<unordered_set>](https://en.cppreference.com/w/cpp/header/unordered_set)
Defined in header [<vector>](https://en.cppreference.com/w/cpp/header/vector)
Opened a PR: https://github.com/InfiniTimeOrg/InfiniTime/pull/2128