SlimeVR / SlimeVR-Tracker-ESP

SlimeVR tracker firmware for ESP32/ESP8266 and different IMUs
Apache License 2.0
798 stars 274 forks source link

Clean up packet bundles and feature flags #294

Open TheDevMinerTV opened 9 months ago

0forks commented 9 months ago

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.

image

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.

https://github.com/SlimeVR/SlimeVR-Tracker-ESP/blob/dd2156669d48e38720729d146e1021da9e1a94b3/src/network/connection.cpp#L117-L128

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.

https://github.com/SlimeVR/SlimeVR-Tracker-ESP/blob/dd2156669d48e38720729d146e1021da9e1a94b3/src/network/connection.h#L173-L175

TheDevMinerTV commented 9 months ago

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

Next time please use review comments ^^

TheDevMinerTV commented 9 months ago

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 commented 8 months ago

@0forks @unlogisch04 @Eirenliel please review

unlogisch04 commented 1 week ago

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