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

Allow the same report ID in multiple input/output/feature reports #833

Closed
wants to merge 9 commits into from

Conversation

afpineda
Copy link
Contributor

@afpineda afpineda commented Jan 3, 2025

Related to issue 279 in esp-nimble-cpp.

What fixes:

  • The user gets a different characteristic for different report IDs or report types.
  • The user gets the same characteristic for the same report ID and report type.

@h2zero
Copy link
Owner

h2zero commented Jan 4, 2025

LGTM, thanks! Could you please rebase this so I can merge?

@afpineda
Copy link
Contributor Author

afpineda commented Jan 4, 2025

LGTM, thanks! Could you please rebase this so I can merge?

I did an "update from upstream/master" in GitHub Desktop.
Hope it work works just the same.
Git rebase scares me a lot !!!.

@h2zero
Copy link
Owner

h2zero commented Jan 4, 2025

Hope it work works just the same.

Sadly no, it actually creates 9 commits in this case instead of 1.

Git rebase scares me a lot !!!.

It's only scary the first time 😄, it's actually quite easy and safe if you try it locally and don't push anything until it looks right, can always reset to remote if it goes wrong.

I'll just squash this for you in this case though as there are no conflicts that way.

PS: Another way to rebase safely without the rebase command is to create a patch file from the diff, reset --hard origin/master, then apply your patch file. Github will even create a patch for you like this: https://github.com/h2zero/NimBLE-Arduino/pull/833.patch

@h2zero
Copy link
Owner

h2zero commented Jan 4, 2025

Closing this as it has been rebased and merged in 9864abe Thanks!

@h2zero h2zero closed this Jan 4, 2025
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