Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

I might try to get this to run on some real hardware in a bit just to play with it - but testing the size, the unlinked static archive compiled for an RP2350 microcontroller is only 262 KB of flash with 5.4 KB of RAM usage, which is really really good. In a real application, DCE will bring this even lower. Also, the lunar and high-precision nutation functions are ~50% of the total binary size.

Hi Kiran, yes, it's a pretty compact library, with very little in the way external deps (especially once you get rid of libc). Nutation and the Moon models have relatively large data tables with ~15,000 sets of coefficients each, so it's expected they should take up a big chunk of the compiled code.

One comment I have is that unlike the C++, or the CALCEPH options, which are add-on options to the typical build, the LIBC option should be an explicitly subtractive option. So, I'd suggest to use naming accordingly, either as WITHOUT_LIBC (matching the existing the CMake WITHOUT_CURL) or DISABLE_LIBC instead. Maybe it's not relevant now. After the PR, I'll probably spend a bit of time to come up with a consistent nomenclature for the new build options (and feature macros) introduced for v1.7...

Hi @kiranshila,

I wanted to ask your opinion about having the NOVAS_ in front of the feature macros. My take is that it's really not needed here. These feature macros don't sit in a global namespace. They are used only locally, at build time, and set only for the relevant build targets. As such, I'd opt for simpler names without the NOVAS_ prefix. Maybe you have a counter point to that?

One comment I have is that unlike the C++, or the CALCEPH options, which are add-on options to the typical build, the LIBC option should be an explicitly subtractive option. So, I'd suggest to use naming accordingly, either as WITHOUT_LIBC (matching the existing the CMake WITHOUT_CURL) or DISABLE_LIBC instead. Maybe it's not relevant now. After the PR, I'll probably spend a bit of time to come up with a consistent nomenclature for the new build options (and feature macros) introduced for v1.7...

Makes sense! I'll update that then.

Hi @kiranshila,

I wanted to ask your opinion about having the NOVAS_ in front of the feature macros. My take is that it's really not needed here. These feature macros don't sit in a global namespace. They are used only locally, at build time, and set only for the relevant build targets. As such, I'd opt for simpler names without the NOVAS_ prefix. Maybe you have a counter point to that?

Yeah because they're scoped appropriately I don't see the issue I think?

Hi Kiran,

I plan to merge this tomorrow, unless you want me to hold off until you do your bare metal testing. (I'm not in any rush, but I also don't want to delay it for no reason.) ;-)

Alright give this a check again, I think I switched enough things after mucking with the rebase