Allow an application to reject a write from onWrite (return an ATT error to the client)

[Nicba1010]

Allow an application to reject a write from onWrite (return an ATT error to the client)

Right now there's no way for a NimBLECharacteristicCallbacks::onWrite() handler to reject a write and have the peer actually find out about it. onWrite() returns void, and the GATT access handler hardcodes a success return on the write path, so no matter what the application does inside the callback, the client always gets an acknowledgement that the write succeeded.

The underlying C stack is perfectly happy to send an ATT error here — the wrapper just doesn't give application code a way to ask for one. I'd like to close that gap.

Checked against master @ 1eddc28515bb (also present in the 2.5.0 release).

Where it gets dropped

NimBLEServer::handleGattEvent is the ble_gatt_access_fn registered with the host. Its read path forwards an ATT error when the wrapper itself can't satisfy the request, and the length guards reject oversize writes — but the write path throws away whatever the callback did:

// src/NimBLEServer.cpp, handleGattEvent()
return rc == 0 ? 0 : BLE_ATT_ERR_INSUFFICIENT_RES;          // read path forwards an error
if (len > maxLen) return BLE_ATT_ERR_INVALID_ATTR_VALUE_LEN; // length is rejected for us

pAtt->writeEvent(buf, len, peerInfo);   // application callback runs here...
return 0;                               // ...and its outcome is discarded

writeEvent() and the callback it calls are both void, all the way down to the base attribute:

// src/NimBLECharacteristic.cpp
void NimBLECharacteristic::writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& connInfo) {
    setValue(val, len);
    m_pCallbacks->onWrite(this, connInfo);   // nothing can come back out
}

// src/NimBLECharacteristic.h
virtual void onWrite(NimBLECharacteristic*, NimBLEConnInfo&);

// src/NimBLELocalValueAttribute.h
virtual void writeEvent(const uint8_t*, uint16_t, NimBLEConnInfo&) = 0;

So the wrapper surfaces ATT errors it generates itself, but never lets the application generate one on a write.

Is this even supported by the underlying stack? (yes — with one caveat)

When traced through the NimBLE host, a non-zero return from the access callback turns out to be honored — so this isn't proposing something the stack would just ignore:

• ble_att_svr_write() takes the access callback's return and turns it directly into the ATT error code — att_err = rc; (ble_att_svr.c).
• For a Write Request (write-with-response), ble_att_svr_rx_write() feeds that att_err into ble_att_svr_tx_rsp(..., BLE_ATT_OP_WRITE_REQ, att_err, ...), which sends the client a proper ATT Error Response.
• For a Write Command (write-without-response), ble_att_svr_rx_write_no_rsp() discards the result and sends nothing — which is correct, since an unacknowledged write must not be answered.

One caveat worth stating up front: this only does anything for write-with-response. A write-without-response (ATT Write Command) is unacknowledged, so the stack sends nothing back to the peer even on error — that's per spec, not something a fix here could change. It's worth a line in the docs so nobody expects a rejection to surface on a WRITE_NO_RSP characteristic. For the acknowledged case, though, the whole path is already there and works.

Why it'd help

The length and permission cases are already handled. The gap is application-level validation and back-pressure — the kinds of things only the application knows about:

• a bounded, fixed-size queue or ring buffer behind the characteristic is momentarily full, and you'd rather tell the client "not right now" than silently drop its command;
• the payload passes the length check but fails a semantic/content check;
• the device is transiently unable to accept the write and wants the client to retry.

Under write-with-response these are exactly what the ATT error response is for. Today the only ways to get one are to abandon the C++ wrapper for a hand-rolled raw NimBLE C service, or to accept the write and drop the data while the client believes it landed. Neither is great when the acknowledgement is load-bearing.

Two ways to fix it

Option 1 — additive, non-breaking (what I'd suggest, so this can land sooner). Add opt-in status-returning callbacks that default to the current behavior, and thread an int up the internal writeEvent/readEvent chain. Existing code that only overrides onWrite/onRead doesn't change at all:

class NimBLECharacteristicCallbacks {
public:
    virtual void onRead (NimBLECharacteristic*, NimBLEConnInfo&);
    virtual void onWrite(NimBLECharacteristic*, NimBLEConnInfo&);

    // Return 0 to accept, or a BLE_ATT_ERR_* code to reject.
    // Default keeps today's behavior exactly.
    virtual int onWriteStatus(NimBLECharacteristic* c, NimBLEConnInfo& i) {
        onWrite(c, i);
        return 0;
    }
    virtual int onReadStatus(NimBLECharacteristic* c, NimBLEConnInfo& i) {
        onRead(c, i);
        return 0;
    }
};

with the internal plumbing (not app-facing, so a return-type change is safe) becoming:

// NimBLELocalValueAttribute.h
virtual int writeEvent(const uint8_t*, uint16_t, NimBLEConnInfo&) = 0;
virtual int readEvent (NimBLEConnInfo&) = 0;

// NimBLECharacteristic.cpp
int NimBLECharacteristic::writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& i) {
    setValue(val, len);
    return m_pCallbacks->onWriteStatus(this, i);
}

// NimBLEServer::handleGattEvent, write path
return pAtt->writeEvent(buf, len, peerInfo);   // was: writeEvent(...); return 0;

The same treatment extends naturally to NimBLEDescriptorCallbacks.

Option 2 — the clean version, but breaking. Just change onRead/onWrite themselves to return int (0 = accept). It mirrors the C stack one-to-one and there's no second set of methods to explain — but it breaks every existing override, so it really belongs in a major version bump.

My hope is that Option 1 can go in as-is without disturbing anyone, and Option 2 becomes the natural shape whenever the next breaking release comes around. I'm happy to open a PR for whichever you'd rather take — just wanted to agree on the direction first so the API lands right the first time.

One small thing to decide either way: writeEvent() currently commits setValue() before the callback runs (so getValue() works inside onWrite). If a write is rejected, the local value has already been updated. We can either document that, or pass the pending (data, len) into the status callback so the application can validate before anything is committed. I lean toward the latter but don't feel strongly.

Assuming the ESP-IDF component is the primary home for this, the NimBLE-Arduino mirror would want the same change.

---

AI usage disclosure

In the interest of transparency: I used an AI assistant while putting this together — to help trace the call path through the C host stack, cross-check the claims above against the source, and draft the code for the proposed fix. Flagging that so the AI involvement is on the table. The file references and code paths cited are all pointed at specific locations in the sources, so they're straightforward to check against the tree.