-
Notifications
You must be signed in to change notification settings - Fork 180
Allow advertisement to contain multiple types of data #89
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@asharov so far the changes look great, thanks for submitting!
I think we can re-use the structure you created for scan response as well. Let me know your thoughts.
unsigned char type; | ||
unsigned char data[BLE_ADVERTISEMENT_DATA_MAX_VALUE_LENGTH]; | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this!
What do you think about renaming it to BLEEirData
or something else, and also using it for the scan response arguments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, that sounds like a good idea. I can add it to the PR.
unsigned char scanDataLength = 0; | ||
|
||
unsigned char advertisementData[BLE_ADVERTISEMENT_DATA_MAX_VALUE_LENGTH]; | ||
BLEAdvertisementData advertisementData[3]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, this will using about 42 more bytes of RAM on nRF8001 boards, which I think is ok. Not sure if there's a way to reduce it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can only think of having the size of this array in DeviceLimits so it could be different for different boards, setting it to 1 on nRF8001, but I wouldn't know what it should be on nRF51822.
Allow advertisement to contain multiple types of data
@asharov thanks for the contribution, awesome work! |
Per issue #86, implementation as discussed there.