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

Make stream state available for message field callbacks also during message size calculation #884

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

reavertm
Copy link

This is more of a question rather than puill request.

I have simple message with arbitrarily big payload that I want to handle procedurally.
I would also like to use message field callbacks.

Unless I'm mistaken, I need to set option callback_datatype just to get message callbacks prototype generated even though I won't be using those pointers.

message Test {
  required bytes payload = 1 [(nanopb).callback_datatype = "void*"];
}

After implementing Test_callback everything works as expected except my payload size is not known inside the callback so I need to pass it as global variable at the moment.
ostream->state is only set on 2nd callback invocation (in encode mode).
Is it intentional, in ex. to tell user that it's "size calculation phase"? (so that one could just manually update bytes_written or do dummy pb_write(... NULL, size) instead of doing more sophisticated encoding)
If so, is there any way to pass user data than global variable?
One could also of course just encode message manually using encoding primitives functions, but I would prefer to use message field callbacks as much as possible (manual set callbacks are ..meh)

Message field callbacks don't get arg, so global variables are the only way to pass anything at the moment.
@PetteriAimonen
Copy link
Member

Sizing streams are identified by stream->callback == NULL, e.g. in here.

So I agree that there seems to be no reason not to make the state available.

It would make sense to do the same in pb_get_encoded_size().
Ideally this pull request would also add a check to tests/encode_unittests, but if that seems complex, I can do it myself later.

@reavertm
Copy link
Author

It seems pb_{i,o}stream_from_buffer() functions reuse stream state themselves.
I assume you would intend to keep it that way and not have dedicated additional let's say void* arg in stream structures, untouched by nanopb itself?

@PetteriAimonen
Copy link
Member

@reavertm I'm not sure. I can see that having extra arg there would make sense in this case, but I'm not sure if it gets a bit strange to mix message state with stream state. E.g. pb_get_encoded_size() doesn't have a way to pass a stream.

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.

None yet

2 participants