Clean up packet bundles and feature flags

[TheDevMinerTV]

No description provided.

[unlogisch04]

I'm not sure about the changes in featureflags. I can't review that. That template/class is something that i understand too less.

The following should be reviewed, i could not read the code from the other side so i dont know wht it expects:
sendPacketType
sendLong (uint64_t or int64_t)

[0forks]

The new packet bundling logic is not working as intended, sends empty bundles flooding the network when there's no data written between beginBundle() and endBundle(). Idle PPS is expected be around 5-10, I'm observing close to 1000.

I mentioned previously that we can't write the bundle header to the UDP library buffer from beginBundle() because an API to reset it doesn't exist. I suggest reverting to the original logic or making a dynamic buffer (or just large enough: 64 * MAX_IMU_COUNT) to hold the whole bundle, which is slightly less efficient. Either way it'll look messy I think.

SlimeVR-Tracker-ESP/src/network/connection.cpp

Lines 117 to 128 in dd21566

	bool Connection::beginBundle() {
	memset(m_BundledPacket, 0, sizeof(m_BundledPacket));
	m_BundlePacketInnerCount = 0;
	MUST_TRANSFER_BOOL(m_ServerFeatures.has(EServerFeatureFlags::PROTOCOL_BUNDLE_SUPPORT
	));
	MUST_TRANSFER_BOOL(m_Connected);
	MUST_TRANSFER_BOOL(!m_IsBundle);
	MUST_TRANSFER_BOOL(beginPacket());
	MUST_TRANSFER_BOOL(sendPacketType(PACKET_BUNDLE));
	MUST_TRANSFER_BOOL(sendPacketNumber());

We could alternatively fix it by changing checks in the Sensor/SensorManager, but that would be less flexible, for example, if we want to wrap more than just sensor->sendData().

Maybe also worth moving the list of default firmware flags back into featureflags.h so it's more obvious that they can be marked as enabled when adding new features.

SlimeVR-Tracker-ESP/src/network/connection.h

Lines 173 to 175 in dd21566

	std::unordered_map<EFirmwareFeatureFlags, bool, EnumClassHash>
	m_EnabledFirmwareFeatures
	= {{EFirmwareFeatureFlags::B64_WIFI_SCANNING, true}};

[TheDevMinerTV]

I've moved the list of active features next to where they are declared.

Next time please use review comments ^^

[TheDevMinerTV]

The new packet bundling logic is not working as intended, sends empty bundles flooding the network when there's no data written between beginBundle() and endBundle().

I fixed this now, it how has a buffer of the size of MAX_IMU_COUNT * 53 (53 being the size of the inspection packet).

[TheDevMinerTV]

@0forks @unlogisch04 @Eirenliel please review

[unlogisch04]

For me it seems to work. But i did not test it with trackers.
For ICM20948 there was a fix needed as they had a internal delay to not send too much updates.
For me the featureflags is something that could be improved more in the meaning more simple as 1 class / function that is seeded with different enums.

[unlogisch04]

This todo is open?
Forgot or does it not matter for this PR?

[unlogisch04]

Clean up and reset of the buffer?
I think we should check for length before we even add more bytes, so we could send a packetbundle, and open a new one for the one that did not fit.

[unlogisch04]

I do not like the template here. As we do not save any programming bytes with it. Also the code understanding is more difficult in my meaning. I think that's a possible improvement for the future.

[ButterscotchV]

Looks fine, I am curious about the comments on the last review though and would appreciate an update

[0forks]

Please change to MAX_IMU_COUNT * 64 (really 63 but it'll be aligned anyway), it'll drop packets in case of BNO+mag+tap with only 55 bytes. The comment also should reflect that it's the size of the whole bundle, not individual packets inside.

[unlogisch04]

I did check the performance on that PR. It seems todo the relevant function (has) is about 8 times slower: It only gets called currently at each packet once. so about 100-120times a sec.

FeatureFlag1 = the new function
FeatureFlag2 = existing function
FeatureFlag3 = Raw && 2 uint8_t > 0

FeatureFlag1 Setuptime: 9 us
FeatureFlag1 100000     reads: 81500 us
FeatureFlag1 1      avg reads: 0.815 us
FeatureFlag2 Setuptime: 1 us
FeatureFlag2 100000     reads: 11258 us
FeatureFlag2 1      avg reads: 0.113 us
FeatureFlag3 Setuptime: 4 us
FeatureFlag3 100000     reads: 10049 us
FeatureFlag3 1      avg reads: 0.100 us