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

Refactor to type-safe data-api #2667

Merged
merged 1 commit into from
Jun 6, 2024
Merged

Refactor to type-safe data-api #2667

merged 1 commit into from
Jun 6, 2024

Conversation

zuckschwerdt
Copy link
Collaborator

There are a few example use-cases now that use the protocol decoders.
This entails using the whole data_t plumbing and some of the outputs.
We should make it easier to decouple data output in decoders from the current implementation details.

This can be achieved by moving from one big varargs list in data_make() to individual data_append_x() calls.
(Users of the decoders can then easily provide their own stubs for these simple functions.)

The changes to bresser_3ch.c are a simple sketch how this could look (other changes just to have it compiling).
(Likely the final type used/passed would not be *data_t / *data but a wrapper type)

Pros:

  • Syntax checking for each output
  • Semantic help from IDEs
  • Flexible prototypes for the calls (e.g. we could pass something like two ints to represent a complex number ;)
  • Easier to extend (the calls do not need to look that much like the resulting data structure)
  • (no vararg magic)

Cons:

  • maybe a slight overhead from issuing ~10 calls instead of one big vararg call (this is offset by not dealing with varargs setup/teardown though)
  • Need to bulk replace existing data_make()s

@merbanan
Copy link
Owner

Can you elaborate more about other use cases ?

@zuckschwerdt
Copy link
Collaborator Author

Can you elaborate more about other use cases ?

I have been toying around writing tools that use the decoders. When binding to languages other than C the varargs don't play nice. E.g. Rust is very C friendly but linking the decoders won't work well if I opt-out of our output backends.

One example would be a test and auto documentation suite that runs pulse data through decoders: I don't want the usual output but to collect structs that I can then use to generate png's show the flow of data bits and such.

One other example is the long-standing request from Sonoff RF Bridge users running the Portisch firmware to pipe their pulse data through the decoders by way of MQTT. It is somewhat possible, but ideally it could be sleaker as just slicers and decoders are needed.

@zuckschwerdt
Copy link
Collaborator Author

I've rewritten this to replace data_append() with the type-safe alternative.

E.g.

data_append(data,
        "message_num", "Message repeat count", DATA_INT, events,
        NULL);

now is simply

data = data_int(data, "message_num", "Message repeat count", NULL, events);

All data_append() calls have been rewritten to show-case the new API and clean up the code.

This also enables us to provide convenience helpers like the example data_hex() which appends a string formatted from a raw array (uint8_t *).
This future-proofs us to add more such helpers for common situations in decoders.

The basic vdata_make() is unchanged and data_append() is still used internally. This can later be updated.

@zuckschwerdt zuckschwerdt marked this pull request as ready for review April 7, 2024 18:37
@zuckschwerdt zuckschwerdt merged commit 5ff470d into master Jun 6, 2024
12 checks passed
@zuckschwerdt zuckschwerdt deleted the wip-dataapi branch June 6, 2024 11:10
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