Hi Marcel, > I am feeling uneasy about doing it this way. Can we try to start with just using a uint8 for the ATT error and see how far that gets us. I have bad experience with overloading values from standards and binary protocols with our own. And I know range wise this is safe (at least at the moment). > > Or for that case we have an extra parameter indicating the success or failure of a verified write. It might be better to make it procedure specific instead of generalizing it. > > We do not have to do it all at once. This is why I think we should start just reporting the ATT errors and see how far that gets us. So we learn which are the cases that will actually need special handling. > > For the cases of protocol errors, there is not much point in telling the higher layers about the exact violation. If it happens, you are in trouble. Nothing you do will fix it. > Now that I look at it, there are 4 errors that I have defined: - UNKNOWN, which is only returned when a malloc fails in a call to bt_att_send. - INVALID_RSP, returned when the server sends a malformed PDU - FAILED_ALLOC, returned when a malloc fails in bt_gatt routines/subroutines - RELIABLE_WRITE, returned when the reliable write verification fails The first 3 of these have no good way of recovering from other than killing the connection or crashing/exiting and the last one applies to only one specific GATT procedure. So I'll go with your suggestion here. All existing callback functions will accept a "bool success" and a "uint8_t att_ecode" parameter. If att_ecode is 0 but success is false, then one of the first 3 conditions above happened. For reliable writes I'll add a new callback which also takes in a "bool reliable_verified", that if false, indicates that the write failed because it couldn't be verified. The other case that I can think of is the signed write, for which we can have a specific callback as well. Sound good? >> I think so. I like to keep these procedures more or less inline with >> what is defined in the core spec. The distinction between these is >> simple enough that the upper layer can correctly determine which one >> to use as needed. > > We can do that of course, but for me the questions is always if it gets really used that way. Will any code really make the distinction and care about it. If not, then this is just bloat that makes the higher layers work for something they should not. > I see value in having these separate routines. I will keep them for now, but I'd be happy to remove/revise them later if unifying them turns out to be better as the whole thing starts getting clearer as I work on it. Cheers, Arman -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
