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
BT: Host: Notification dropped instead of truncated if bigger than ATT_MTU-3 #23134
Comments
Despite what the spec says, silent truncation of data (which in most cases will result in corrupt data) doesn't sound like a smart idea. We could make this "non-silent" on our side by changing the My current feeling would be to give this some time so we can discuss the options and look for some solution for 2.3. To my understanding this isn't breaking any qualification test case so I wouldn't consider it that critical. |
Based on some further discussion on Slack, I can understand the opposing argument as well, i.e. keep the GATT server simple and leave it to the client to check MTU and use another procedure to get more data. So please don't stop the creation of a PR just because of my earlier comment :) |
Id leave the implementation as it is, Id never seem any profile spec that does require reading after notifying since that is racy, in fact that could be a bug in the upper layer that don't check the MTU or haven't triggered a MTU exchange. |
Since there is some contention on how to proceed with this, I don't think it's appropriate to consider it a medium priority bug that we should try to "fix" for 2.2. I'd rather us spend some more time thinking how to best fix this, if a fix is needed to begin with. From an API perspective, even if we choose to do truncation, it should be possible for the application to notice from the |
Changed to low priority because:
|
Note that this issue applies to both notifications and indications. @Vudentz I think Zephyr should handle this to support the use-case of bt_gatt_notify with conn=NULL, which sends the notification to all connected devices that are subscribed, as each of those devices could have a different MTU. Given the above use-case, instead of changing the return value of bt_gatt_notify, we should probably indicate the truncation or number of bytes sent in one or more of the callbacks (cfg_match and/or gatt_complete_func) so the application is aware of it. The cfg_match callback would be a nice one for this as it would allow the application to reject the truncated notification and try again after negotiating MTU. In any case, I agree that this needs further thoughts before a PR, and such an API change can not be done at this late stage. |
Given the bt_gatt_notify_multiple PR by @Vudentz I don't think that having it as a return value is a solution. Maybe we should just have a kconfig option to enable the behavior? |
I would agree with a Kconfig to enable truncation. Should we also make an API change so that the number of bytes sent is sent to the cfg_match callback? |
I don't think we should do that. The number of bytes sent could be inferred by using |
From slack:
The text was updated successfully, but these errors were encountered: