Additional Device Informations for ble #336
Conversation
| }; | ||
|
|
||
| public: | ||
| BLEDis(void); | ||
|
|
||
| void setModel(const char* model); | ||
| void setModel(const char* model,uint8_t length); |
There was a problem hiding this comment.
for using this API with length, I would suggest to
- change the type of 1st parameter to
const uint8_t*e.gsetModel(const uint8_t model, uint8_t length) - also please implement the
const char*as inline function as well.
There was a problem hiding this comment.
- Yes, you are right. Then inside function it must be "_model = (const char*)model;"
- Sorry, I don't know what to change, or do you mean "_model = (const char*)model;"
| chars.setFixedLen(strlen(_strarr[i])); | ||
|
|
||
| if (_strarr_length[i]) { | ||
| chars.setFixedLen(_strarr_length[i]); |
There was a problem hiding this comment.
- should only use the _strarr_legnth[i], there is no point to use the strlen() function anymore.
There was a problem hiding this comment.
Yes, you are right.
| VERIFY_STATUS( chars.begin() ); | ||
| chars.write(_strarr[i]); | ||
| if (_strarr_length[i]) { |
There was a problem hiding this comment.
Yes, you are right.
| } | ||
| } | ||
|
|
||
| if ( _strarr[arrcount(_strarr)-1] != NULL ) |
There was a problem hiding this comment.
- I am not familliar with PNP_ID, but why this characteristic isn't included in the above loop
There was a problem hiding this comment.
Last characteristic from the loop is 0x2A2A (IEEE_REGULATORY_CERTIFICATION), PNP_ID is 0x2A50. So there is a big gap between last entry from loop and PNP_ID. I had no better idea than to do it seperatly.
There was a problem hiding this comment.
@elral you should update your code then commit and push following my review. However, since you are new to PR, I will merge this and do the clean up myself. Thank you very much for the PR. Looking forward to more of your contribution in the future.
- inline function for no-length API - clean up using strarr_len - handle PNP_ID char
Added some methods for setting more device informations.
See also Issue #335