linxGnu / gosmpp

Smpp (3.4) Client Library for Go
Apache License 2.0
152 stars 59 forks source link

new OnAllPDU seeting that lets users choose to handle all PDUs from SMSC #110

Closed laduchesneau closed 1 year ago

laduchesneau commented 1 year ago

As propose in #96 and in #107

What: This feature allows the ESME to control PDU response by using a new setting that will handle all PDU. This proposed feature does not interfere with current user behaviour.

Why: Some ESME need to validate incoming PDUs, like DeliverSm and DataSM, before returning a response.

How:

Warning: This setting, if set, overrides OnPDU and bypass all auto response feature, Enquirelink and Unbind request must be handled by user.

Added a new example called transcceiver_with_manual_response to see how it can be used, moved old example in transceiver_with_auto_response .

laduchesneau commented 1 year ago

I never like adding func (t *receivable) Respond(p pdu.PDU) error to both receivable and transceivable. What if OnAllPDU had a return vlaue ?

OnAllPDU func(pdu pdu.PDU) pdu.PDU

And receivable just check if the return pdu wasnt nil.

if t.settings.OnAllPDU != nil {
    r := t.settings.OnAllPDU(p)
    if r != nil {
        t.settings.response(r)
    }
    return
}

I like this better.

linxGnu commented 1 year ago

@laduchesneau Yes, I like the second approach too:

if t.settings.OnAllPDU != nil {
    r := t.settings.OnAllPDU(p)
    if r != nil {
        t.settings.response(r)
    }
    return
}
laduchesneau commented 1 year ago

Return value added to OnAllPDU, I like this a lot more, code is simpler.

Removed a lot of change from initial commit in PR. When/if it gets merge, recommend squashing all commits.

linxGnu commented 1 year ago

LGTM. Could you please have a look too @goten4