crownstone / bluenet

Bluenet is the in-house firmware on Crownstone hardware. Functions: switching, dimming, energy monitoring, presence detection, indoor localization, switchcraft.
https://crownstone.rocks
91 stars 62 forks source link

Builtin two support #149

Closed mrquincle closed 2 years ago

mrquincle commented 2 years ago

Support for the Crownstone built-in two. This also rewrites much about how the boards are configured. It also explains a lot of the values in cs_Boards.c.

mrquincle commented 2 years ago

Especially https://github.com/crownstone/bluenet/blob/1cc9401b3a291c51314e78d10734b763356757b0/source/include/cfg/cs_Boards.h has lots of additional comments which are useful when adding new boards (or checking the configuration of the current ones).

vliedel commented 2 years ago

There is more that can be improved, but will not be included in this PR:

ArrowAcrobatics commented 2 years ago

cs_boards.h

cs_Boards.c

main.c (bootloader)

cs_crownstone.cpp

cs_hardwareVersions.h

vliedel commented 2 years ago
change board name -> int defines into enum (enum class)

That would break load_hardware_version_mapping.cmake. And we can't make this a class, it's C code.

add unit to variable names. E.g. `uint32_t voltageRange;' mV uint32_t currentRange; this says mV in comment... but it is the voltage range on the adc pin measuring current? maybe rename uint32_t voltageZero[GAIN_COUNT]; adc measurement units

We can improve the names and explanation a bit more indeed.

line 210: why is board configured asACR01B2C(config); ragher than g_DEFAULT_HARDWARE_BOARD in default case?

Good point, I'm not sure if we ever end up here, but what to do in case we do? A recursive call could result in endless recursion if the default board is invalid. Doing nothing might be the best solution?

Change return code from an NRF error to one from cs_ErrorCodes.h?

Yes.

an idea to reduce flash program size: instead of writing a giant setter function, define the boards as static (global) constants
and just assign a reference/pointer to that. Should be a minor change in code I think.

Setting default values would be harder, especially for arrays. You would have to set every value for every board instead of only overwriting some values. I'm not sure this would end up reducing the program size.

cs_gpio_init: switch board for nfc pins can be replaced with config->flags.usesNfcPins?

Guess i forgot about the bootloader.

is CS_TEST_PIN still relevant (line 72)

Doesn't look like it, but doesn't have much to do with the board config change =]

cs_hardwareVersions.h could make this into a switch and remove commented code

Ah, hadn't looked at this, but yeah.

vliedel commented 2 years ago

Implemented.

mrquincle commented 2 years ago

Can this one be merged and closed?