usddeck: Add eventriggerType_fp16

[jonasdn]

Fixes: #798

[whoenig]

Do we actually have variables of this type? Your code changes look good, but I am worried about proper support on the host side. In particular, does struct.decode do the right thing and is the firmware actually using the IEEE 754 binary16 standard? Supporting this datatype on a C++ host might also be challenging, as this is none of the standard types in C(++).

[jonasdn]

Do we actually have variables of this type? Your code changes look good, but I am worried about proper support on the host side. In particular, does struct.decode do the right thing and is the firmware actually using the IEEE 754 binary16 standard? Supporting this datatype on a C++ host might also be challenging, as this is none of the standard types in C(++).

Good questions!

Python supports this since I think 3.6: https://docs.python.org/3/library/struct.html#format-characters, and I believe it should work. And according to @ataffanel this should work (https://forum.bitcraze.io/viewtopic.php?p=22023&sid=311d5a4e46d027554d6e04312f573ebd#p22023).

I think both clang and gcc has extensions to support this. But! Your objections are fair. Perhaps instead of this PR we make it not assert and instead warn in some way?

[whoenig]

My (bold) suggestion would have been to just remove FP16 entirely from the firmware - less to test & maintain... I don't remember seeing it being used inside the firmware and I've also not really seen the feature being used to log data with a different type (e.g., logging a float32 variable as FP16 to fit more inside a single log block.)

[ataffanel]

There is currently no variable that uses FP16 in the firmware memory, this is only converted to be transferred in the log packets.

My intention was to implement IEEE 754 binary16 when I implemented FP16 in the Crazyflie. From my tests, the python implementation works fine with it. I do not remember being too careful about rounding and subnormalized representation is not handled, but for the purpose of compressing floating point representation it works well. For reference, the implementation is there:

crazyflie-firmware/src/utils/src/num.c

Lines 48 to 62 in a70c357

	uint16_t single2half(float number)
	{
	uint32_t num = *((uint32_t*)&number);
	uint32_t s = num>>31;
	uint32_t e = (num>>23)&0x0FF;
	if ((e==255) && (num&0x007fffff))
	return 0x7E00; // NaN
	if (e>(127+15))
	return s?0xFC00:0x7C00; //+/- inf
	if (e<(127-15))
	return 0; //Do not handle generating subnormalised representation
	return (s<<15) | ((e-127+15)<<10) | (((num>>13)&0x3FF)+((num>>12)&0x01));
	}

.

Now, when it comes to the SD-card deck, it may be that fp16 is not that interesting since we have a lot more bandwidth. Though if it can be handled rather than failing it would be more coherent and less surprising if anyone tries to use it.

GCC support for fp16 was added right after I made my own implementation, it might also be an idea to use it in the Crazyflie so that we can be a bit more confident with interoperability.

[ataffanel]

My (bold) suggestion would have been to just remove FP16 entirely from the firmware - less to test & maintain... I don't remember seeing it being used inside the firmware and I've also not really seen the feature being used to log data with a different type (e.g., logging a float32 variable as FP16 to fit more inside a single log block.)

It is implemented in the Crazyflie lib now. It was implemented after a forum request to make this packet possible using a single log block:
https://github.com/bitcraze/crazyflie-lib-python/blob/master/examples/basiclog.py#L75-L84

I do think that it is a useful functionality when it comes to the radio since we have such a limited space in each log packet.

[whoenig]

For this particular case, I think it makes more sense to use pm.vbatMV (

crazyflie-firmware/src/hal/src/pm_stm32f4.c

Line 421 in 6b04364

LOG_ADD(LOG_UINT16, vbatMV, &batteryVoltageMV)

).

But going back to the topic at hand: if fp16 is to remain supported, then this PR looks good.