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

Definition for known format types. #230

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

Conversation

mklemarczyk
Copy link
Contributor

@mklemarczyk mklemarczyk commented Apr 6, 2022

To simplify usage of the ArduinoBLE, I added the definitions for format types.
In the future I would like to add also known services and characteristics uuid codes.

I check with the Arduino compiler that the definitions are included in result binary only if they are used by the code. So no extra space will be taken, Please review and tell what do you think about the idea. I find it more useful than searching the internet for proper values.

I has been forced to use char type, as Description constructor does not accept the uint8_t type as input for value.
BLEDescriptor::BLEDescriptor(const char* uuid, const uint8_t value[], int valueSize)
BLEDescriptor::BLEDescriptor(const char* uuid, const char* value)

@CLAassistant
Copy link

CLAassistant commented Apr 6, 2022

CLA assistant check
All committers have signed the CLA.

@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Apr 7, 2022
@rmlearney-digicatapult
Copy link

rmlearney-digicatapult commented Jun 24, 2022

IMHO would it look better if these were snake_case rather than CamelCase, and made to resemble the C99 standard:

BleUint8 -> ble_uint8_t
BleInt8 -> ble_int8_t
BleUint64 -> ble_uint64_t

etc.

But probably irrelevant for this.

@mklemarczyk
Copy link
Contributor Author

mklemarczyk commented Jun 24, 2022

It is a constant for a value, you can not declare those types in the code. Will it not create a confusion with the value types form C99 ? The current naming follows the other constants like BLEEncryption, BLERead.

I would like to ask if there is a concern for merge that PR that I can address? It is waiting quite a long time.

@mklemarczyk
Copy link
Contributor Author

@facchinm You do not want this change? There are several PRs waiting, and no comment or merged.

@mklemarczyk
Copy link
Contributor Author

Bump, any answer on the integration ? I wonder why it takes so long...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants