Interactions-HSG / uArm-Controller

MIT License
1 stars 0 forks source link

Proper pb_decode() error handling #15

Closed iomz closed 3 years ago

iomz commented 3 years ago

In src/protobuf_helper.cpp, potential erroneous PB messages will not be captured there and can be a hidden culprit when debugging the communication with gRPC server.

jo-bru commented 3 years ago

As described in the Nanopb: Migration Dokumentation, messages terminated by a null byte raise an error by default. Therefore I switched to use the pb_decode_ex() function instead. We may need to reconsider the termination by a null byte since this forbids sending fields with the value 0. But for now, it seems to work :)

iomz commented 3 years ago

Aha, cool.

since this forbids sending fields with the value 0.

Is it so? The null byte (0x00) and the ASCII value zero (0x30)are different. (i.e., this or pb_decode_nullterminated() seem good standing)

jo-bru commented 3 years ago

Yes, I meant the null byte (0x00). That's only a problem for the raw byte fields of the DATA response. For the moment, I simply made sure, that no null bytes are used for any drivers. This works but is sometimes a kind of a dirty workaround. For example the implementation of the Ultrasonic_sensor: https://github.com/Interactions-HSG/uArm-Controller/blob/3b2386e93866aa575230fd744c8059cdb57cc27a/src/drivers/ultrasonic_sensor.cpp#L45-L47 Maybe there's a better way, but this is IMO the simplest way.