Skip to content

Add nanopb plugin#1234

Merged
mfridman merged 3 commits into
bufbuild:mainfrom
inomotech-foss:nanopb
May 15, 2024
Merged

Add nanopb plugin#1234
mfridman merged 3 commits into
bufbuild:mainfrom
inomotech-foss:nanopb

Conversation

@scootermon

@scootermon scootermon commented May 11, 2024

Copy link
Copy Markdown
Contributor

Closes #683

This PR is marked as a draft because of the following open points:

  1. still need to successfully run the tests (ran into issues with my devcontainer setup)
  2. nanopb has a runtime dependency on a few c files. Since there isn't really an established library ecosystem for C (and especially not in embedded circles where you would use nanopb) we need to find an alternative like copying the files to the output. Does buf already have a way to deal with this?

@CLAassistant

CLAassistant commented May 11, 2024

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@pkwarren pkwarren left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks great - just a few suggestions on the Dockerfile.

Comment thread plugins/community/nanopb/v0.4.8/Dockerfile Outdated
Comment thread plugins/community/nanopb/v0.4.8/Dockerfile Outdated
Comment thread plugins/community/nanopb/v0.4.8/Dockerfile Outdated
Comment thread plugins/community/nanopb/v0.4.8/Dockerfile Outdated
@pkwarren

pkwarren commented May 14, 2024

Copy link
Copy Markdown
Member

nanopb has a runtime dependency on a few c files. Since there isn't really an established library ecosystem for C (and especially not in embedded circles where you would use nanopb) we need to find an alternative like copying the files to the output. Does buf already have a way to deal with this?

Are you talking about using this plugin as a Generated SDK? If so you are correct and there isn't support for C programs today. This plugin would be available initially only as a Remote Plugin.

If you wish for the plugin itself to include additional C files in the output, it would need to ship them in the nanopg generator plugin itself using the file field on CodeGeneratorResponse: https://github.com/protocolbuffers/protobuf/blob/396d6617674622bf9be4b73088f2f17e9f6eb374/src/google/protobuf/compiler/plugin.proto#L179.

scootermon and others added 2 commits May 15, 2024 11:17
Co-authored-by: Philip K. Warren <pkwarren@users.noreply.github.com>
@scootermon scootermon marked this pull request as ready for review May 15, 2024 09:19
@scootermon

Copy link
Copy Markdown
Contributor Author

Are you talking about using this plugin as a Generated SDK? If so you are correct and there isn't support for C programs today. This plugin would be available initially only as a Remote Plugin.

That's fine then, I just wanted to make sure I would add it as well in case it's already supported by nanopb.

If you wish for the plugin itself to include additional C files in the output, it would need to ship them in the nanopg generator plugin itself using the file field on CodeGeneratorResponse

Makes sense, that's something that would have to be implemented in nanopb itself then.


I went ahead and applied your suggestions, updated the PR, and marked it as ready.

@mfridman

Copy link
Copy Markdown
Member

lgtm, thanks for taking the time to contribute 🥳.

@mfridman mfridman merged commit 2091af3 into bufbuild:main May 15, 2024
@scootermon scootermon deleted the nanopb branch May 15, 2024 13:35
@scootermon

Copy link
Copy Markdown
Contributor Author

Thanks guys!
You both made this a great contributing experience and I really appreciate that!

@mfridman

Copy link
Copy Markdown
Member

Awesome, the plugin has been published via the automation in this repository to https://buf.build/community/nanopb

Took it for a spin

# buf.gen.yaml
version: v1
managed:
  enabled: true
plugins:
  - plugin: buf.build/community/nanopb:v0.4.8
    out: gen

And then generated some code

buf generate buf.build/acme/petapis --include-imports

output

gen
├── google
│   └── type
│       ├── datetime.pb.c
│       ├── datetime.pb.h
│       ├── money.pb.c
│       └── money.pb.h
├── payment
│   └── v1alpha1
│       ├── payment.pb.c
│       └── payment.pb.h
└── pet
    └── v1
        ├── pet.pb.c
        └── pet.pb.h

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.

Plugin request for Buf Schema Registry: nanopb

4 participants