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

examples: add blecent and bleprph similar to mynewt-nimble apps #2807

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

Conversation

maxikrie
Copy link

Summary

These example apps create a BLE central and peripheral based on mynewt-nimBLE and work in companion. The apps are mostly based on their equivalents, which can be found at https://github.com/apache/mynewt-nimble/tree/master/apps

Impact

Testing

These were successfully tested on two nrf52840-dk boards.

These example apps create a BLE central and peripheral based on
mynewt-nimBLE and work in companion. The apps are mostly based
on their equivalents, which can be found at
https://github.com/apache/mynewt-nimble/tree/master/apps
Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

@cederom cederom left a comment

Choose a reason for hiding this comment

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

Comment on lines +31 to +32
void
print_bytes(const uint8_t *bytes, int len)
Copy link
Contributor

Choose a reason for hiding this comment

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

@maxikrie I think it can fit in a single line

}

void
print_mbuf(const struct os_mbuf *om)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

Comment on lines +47 to +55
while (om != NULL) {
if (colon) {
printf(":");
} else {
colon = 1;
}
print_bytes(om->om_data, om->om_len);
om = SLIST_NEXT(om, om_next);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please run the nxstyle or checkpatch.sh, it doesn't adhere to coding style, more info:
https://nuttx.apache.org/docs/latest/contributing/index.html

Copy link
Author

Choose a reason for hiding this comment

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

I will make changes and adhere to the standard. I am sorry that I didn't follow it immediately, it has been a while since my last pull request and I didn't see the documentation on contributing beforehand.

}

char *
addr_str(const void *addr)
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

}

void
print_uuid(const ble_uuid_t *uuid)
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@raiden00pl
Copy link
Contributor

why not modify the current nimble peripheral example (https://github.com/apache/nuttx-apps/blob/master/examples/nimble/nimble_main.c)? do we need 2 different examples for this?

@maxikrie
Copy link
Author

maxikrie commented Nov 1, 2024

why not modify the current nimble peripheral example (https://github.com/apache/nuttx-apps/blob/master/examples/nimble/nimble_main.c)? do we need 2 different examples for this?

I was debating this with myself. As stated, I based them on the mynewt-nimble apps and hence the peripheral is quite different from the existing one. I could, of course, modify the existing one losing the tight connection to the mynewt-nimble app. The other options are maintaining the existing one along side my contribution for legacy reasons, or purging the existing one (my preference). What do you think? @raiden00pl

@raiden00pl
Copy link
Contributor

raiden00pl commented Nov 1, 2024

@maxikrie The Mynewt coding standard is far from the NuttX coding standard, so the code will be a lot different anyway. Keeping 2 examples demonstrating the same functionality seems pointless to me. The easiest thing to do would be to change the name of existing examples/nimble to examples/nimble_bleprph (or nimble_prph) and add all your changes there.
Or you can delete examples/nimble but then you will have more work to adapt the Mynewt code to the NuttX coding standard.

# see the file kconfig-language.txt in the NuttX tools repository.
#

config EXAMPLES_NIMBLE_BLECENT
Copy link
Contributor

Choose a reason for hiding this comment

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

what do you think about using the names nimble_cent and nimble_prph ?

@cederom
Copy link

cederom commented Nov 2, 2024

CI checkpatch is now fixed, CI restarted :-)

@cederom
Copy link

cederom commented Nov 2, 2024

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

Successfully merging this pull request may close these issues.

4 participants