Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Binary RPC REPE #577

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Binary RPC REPE #577

wants to merge 4 commits into from

Conversation

stephenberry
Copy link
Owner

This is implementation of the new REPE binary RPC specification using BEVE

@stephenberry
Copy link
Owner Author

@jbbjarnason, I added a unit test that doesn't build yet, but I'll keep adding to it to describe the kind of interface we want.

I'm thinking of adding custom data as a std::vector<std::byte> to the header. This will require a double parse for the header, but I think this makes sense. The double parse allows users to parse the header knowing exactly what it should look like every time. And, it makes it more efficient to skip.

The message itself is still templated on the body, so you can decode/encode straight to memory.

Copy link
Collaborator

@jbbjarnason jbbjarnason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking good

struct header final
{
uint8_t version = 0b00000'110; // the REPE version
bool error = false; // whether an error has occurred
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the error be something else than bool?
How does this member relate to the error struct below


params = {};
msg.header = {};
expect(!glz::read_binary(msg, buffer));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can read_binary return an error with partial as an error code? In terms of networking, so we would know to async_receive more data into the buffer before trying again.

Secondly would it be beneficial to be able to only read the header?

glz::repe::header header{};
glz::read_binary(header, buffer);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants