Reverse IMU I2C address from supplement to full

[Eirenliel]

Reverses IMU address change in defines.h introduced in #322.

The change is reversed for compatibility with web flasher, as well as for future expandability with use cases with more than 2 IMUs. Directly inputing the addresses give flexibility in case any address shifters are involved, or IMUs are wired in different configurations.

[Eirenliel]

@l0ud please review too, ty

[gorbit99]

I don't like this change for multiple reasons:

1. It makes flashing more technical for the average user, they would need to know the IMU address, which is not easy to get in most cases
2. This introduces a possible desync between the IMU address and the IMU itself
3. This complicates tooling, where this change would need to be handled both for branches where this is present and branches where it isn't

Instead I propose the following solution:

1. Change the expected type of the IMU_DESC_ENTRY to std::optional<uint8_t>, this is done by updating buildSensor in SensorManager.h to have that type
2. In SensorManager.h, add the following lines below the include block:

#ifndef PRIMARY_IMU_ADDRESS_ONE
#define PRIMARY_IMU_ADDRESS_ONE std::nullopt
#endif

#ifndef SECONDARY_IMU_ADDRESS_TWO
#define SECONDARY_IMU_ADDRESS_TWO std::nullopt
#endif

3. Handle the possibility of a nullopt value in buildSensor:

	std::unique_ptr<Sensor> buildSensor(
		uint8_t sensorID,
		std::optional<uint8_t> imuAddress,
		float rotation,
		uint8_t sclPin,
		uint8_t sdaPin,
		bool optional = false,
		int extraParam = 0
	) {
		uint8_t i2cAddress = imuAddress.value_or(ImuType::Address + sensorId);
...

4. Make sure that static constexpr uint8_t Address = ...; is present for all IMU sensor classes. This is already mostly in place for all IMUs, except for BNO080.

With these changes in place, the default IMU address will be used by default, but if some more exotic configuration is necessary, that is also possible, all without changing the functionality.

[Eirenliel]

which tooling supports the current system? webflasher isn't, according to @loucass003 No one cared about the tooling when it was introduced. That's why we're reversing it, actually, there is no tooling that's compatible with 0.5.+ because of it.

And yeah if you are filling the defines, you should probably know which IMU you're using and where to look up the address. Otherwise, rely on tooling to fill the address. Which what webflasher already does.

Your solution is a bit better, but once again, WHY.

[gorbit99]

I've certainly been using butterscotch's webflasher to flash my slimes with sfusion based firmware, and a lot of people are actively using that. The functionality behind how addresses worked did change, but defines.h file generation didn't. Webflasher doesn't support changing IMU addresses (since it basically wasn't a thing that could realistically happen).

Knowing which IMU you are using is a very different beast from knowing the exact I2C address it's on, since usually the only place this is mentioned is the datasheet if even there.

My solution aims to replace the huge ifndef chain that was originally in sensoraddress.h with a much cleaner, driver based approach.

[gorbit99]

Does this currently build without the address variable in the bno driver?

Also, codeowners seems to have an error?

[Eirenliel]

Does this currently build without the address variable in the bno driver?

The address is there.

Also, codeowners seems to have an error?

Because @l0ud isn't in the org :nya_a: I'll add later...

[gorbit99]

lgtm