C++: add missing implementations for client advertised schema/encoding

paulsohn commented 1 week ago

Changelog

C++ server/client: add missing implementations for client advertised schema / schema encoding

Docs

Issue #875 (Initial discussions are here)
Liner Ticket FG-9447 (though I can't access to it)

Description

Although foxglove ws protocol allow client to advertise complete schema definition string as well as schema name, current C++ implementation lacks such fields. Particularly:

ClientAdvertisement struct lacks schemaEncoding field,
ClientAdvertisement struct has schema field, but it is never populated (websocket server does not read schema or schema encoding to json)
websocket client is does not serialize schema or schema encoding to json

This PR resolves above problems, and additionally since this is breaking change from unimplemented to implemented, types are decided as following:

schema field type is changed from std::vector<uint8_t> to std::optional<std::string>, as schema fields are passed in as json quoted string, and it's optional
schemaEncoding field type is determined as std::optional<std::string>, the same reason for schema.

Fixes: foxglove/ws-protocol#875 Fixes: FG-9447

defunctzombie commented 1 week ago

@paulsohn Thanks for the proposed fix here - let us know when this is ready for review!

paulsohn commented 1 week ago

@defunctzombie Sure. It is a draft because I got no tests but I don't think further changes might be needed, so feel free to test this as it is (I will do my tests in next week)

paulsohn commented 4 days ago

@defunctzombie Fixed error so now this is properly ready for review.

Added some example in separate branch ( https://github.com/paulsohn/ws-protocol/tree/test-pr876 ). After build, execute make example_server_client_pub and make example_client_pub_json (or ..._protobuf) on separate shell in that order to test.

paulsohn commented 1 day ago

Are you currently reviewing this? If you don't mind, let me know the progress so far

By the way I just thought, it might affect ros-foxglove-bridge as well?

https://github.com/foxglove/ros-foxglove-bridge/blob/main/ros2_foxglove_bridge/src/ros2_foxglove_bridge.cpp

foxglove / ws-protocol