-
-
Notifications
You must be signed in to change notification settings - Fork 137
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
BLERemoteCharacteristic::readValue() does not return the correct value #20
Comments
Troubling indeed, would it be possible to see a log? |
This is the log with the changes, but this does not differ as such from the log without the changes, only the resulting value is correct :D
|
I'm so far unable to reproduce the issue. I've setup a peripheral device with static values similar to what you are trying to read and am able to read them on a client just as they should be. Maybe the peripheral is doing something and responding with 0 values? I don't see anything wrong in your log, I'm assuming this is after you made changes? |
@h2zero Thnx for helping me! Yes the log is after the changes, but the only difference is the two Dutch sentences near the end.
I don't think so, because the data read from the peripheral is correct. It is not passed correctly to the remote characteristic... |
Interesting how you are recieving notifications before you have registered for them. It also seems you get one right when you happen to be reading the value. I wonder if there is something going on there. |
Maybe I sent the wrong log, I was comparing logging with and without callback to pinpoint the possible rootcause of the issue. I'll send a new log tomorrow, it is late here already... |
Digging deeper into this, I found out that the (standard UUID) battery characteristic is correct and the (proprietary UUID) temperature and humity characteristic has the right value in BLEClient, but the wrong value is set in BLERemoteCharacteristic. I provide a MVCE and the right logging below. I also found out that in the original library another function is used to get the characteristic and another callback mechanism. I also saw that in the original library the value is returned in a similar way I checked the (correct) value in BLEClient (see above comment). Maybe I draft a PR for that, if you @h2zero could check if this is correct. Please also check if it has consequences for write(), because I did not use that yet, although I doubt it has any impact. |
Updated MVCE, also reading the battery characteristic:
|
Updated logging, using this library without any changes:
|
This part of the log is strange:
Is this your own custom peripheral device? It looks like when you try to read the temp/humidity characteristic the peripheral sends a notification instead of returning a read response with the data. However the battery characteristic does not, so you get the correct reading. Could you add this:
here to see what data NimBLE returns? |
No this is not a custom device, it is a standard (square, new type) Xiaomi temp/humidity sensor. I also saw that the same call for the read of a characteristic leads to different paths through the program. For the battery this leads to the correct result, for the temp / humidity it does not. I wil try your suggestion later, probably tomorrow. For now, from my own tests I found out that for the humidity and temperature, the right values are here (in NimBLEClient::handleGapEvent)
The assignment to characteristic->second->m_value works to get the right value to BLERemoteCharacteristic::readValue(), but I did not find out yet how to differentiate between the two cases, because this messes up the battery value... WITH above assignment to m_value the right temperature and humidity is in characteristic->m_value while the right battery value is in attr->om->om_data in NimBLERemoteCharacteristic::onReadCB. The code below shows the right temp / humdity for the first call in the first log, and the right battery for the second call in the second log:
Does this make any sense to you? |
Well my thoughts on this are that the device you're connecting to does not support reading properly and was only intended to send notifications of those values. Which makes sense in a way but it should not have it's readable flag set. Would not be the first time an off the shelf device didn't follow the core specification 😄. So for an explanation of what your code change has done is it has populated the remote characteristic value with the data sent in a notification, but if the battery characteristic doesn't send those you won't get the value updated there and you will have to read it. The Unless I'm missing something here, it seems to me to be an incorrectly coded peripheral. For your puropses I would suggest waiting for a notification to obtain the value, then read the battery then disconnect/delete client. |
Hi, the LYWSD03MMC definitely provides hum/temp via notifications and it is working without any problem with your library. You can check this behavior with every BLE-APP on your phone (i.e. BLE Scanner). In contrast the battery is a readValue and BTW it always gives you 99% as it seems to be hard coded ;) |
@Staars Hi Christiaan, it certainly does, but it makes my use case less elegant. I just need the values at regular intervals, and reading the values at those times is more logical than registrating a call back and waiting for the result. But maybe it is the only option... @h2zero Thanks for your extensive analysis. Would it be an option to have the library support such a scheme, reading a characteristic and pick it up from the notification? |
Sorry, i ddint read all posts in this topic, but after reading this one: You are trying to read raw data with function that is using std::string. You need to use getData() instead (its function from old library, dont know if this library got it).
|
@chegewara the function getData() does not exist indeed, neither on the NimBLECharacteristic nor on the BLECharacteristic. I would love to have such a function, do you have example code? The issue is that the peripheral responds in a, according to @h2zero, off specs way. Does the getData() that you have in mind solve that problem? |
Yes, i think it is in BLERemoteCharacteristic. |
@chegewara I meant, it is not (Nim-)BLERemoteCharacteristic. I made a PR #27 for this, but if the functionality is already available, that would be better. |
I think @chegewara was referring to the I am unsure what you are trying to accomplish in your app, just trying to avoid using the callbacks? I don't think that would be realistic considering the nature of the peripheral. BLE only has 3 ways to get data from a server, 1 is a read operation, 2 is notifications, 3 is in advertisement packets. The trouble with this peripheral is it does not respond to a read request with a read response containing the data and instead sends a notification with it. The good news is it sends the notification immediately after the read request, so it seems from your logs, at least you don't have to wait long for it. From what I've seen on other posts about this device is you may be able to extract the data you want from the advertisement packet, but you need to decrypt it. |
getRawData() uses readValue() under the hood, so that leads to the same problem. I try to accomplish simple and easy to maintain code. Reading a characteristic is far more simple than reading it, registering a callback, monitoring if the callback has completed, stopping the callback, using the value*. I realize that reading a characteristic is also async, but this is abstracted away from the sketch by the library, which is a good thing! *I am now testing with a second device, that sets canRead() to false, so I think I must indeed implement this one with a notify callback. |
Solved by PR #49 |
I would love to get this library running, but I think I found another issue...
I already thought it was my problem so I asked question #19 today, but it is an error of the library.
MVCE:
returns 0.00 for temperature and 0 for humidity.
Digging for hours in the library resulted in:
Here the data is still correct. If I add the following line at this exact position:
characteristic->second->m_value = std::string((char*) event->notify_rx.om->om_data, event->notify_rx.om->om_len);
And comment out this line (otherwise the value just sets gets overwritten), the MVCE actually works correctly.
HOWEVER it does not feel good. My gut feeling says that
onReadCB
should indeed read the value fromstruct ble_gatt_attr *attr
, but that value is not correct. At least the data from event->notify_rx.om in NimBLEClient differs from attr->om in NimbleRemoteCharacteristic.I am not familiair enough with BLE and the library to solve this, and I am stuck now, any suggestions would be very welcome!.
The text was updated successfully, but these errors were encountered: