Hi, first of all: Thank you for your contribution!
J�rgen Thomsen wrote: > See below. > > There is partial support for the SMPP optional parameters in Kannel, but > conservative purists like the one below is preventing people from using > them, because not all protocols support them. I'm another one conservative purist like the one below (Stipe). Vote: -1 reasons are good described from Stipe. > > Although there is a point in keeping code clean, the attitude below is in > principle wrong. > > The proper attitude according to my opinion should be, that we have a core > data structure, but as the lowest common denominator definitely is not > good enough, we must add provisions to extend this core in a controlled > manner. In controlled manner? if I look in your patch you propose to define _every_ TLV in message struct? and later for extra things in emi module? and so on... Btw. if you want to mantain this patch on your own then at least smpp version check is approriate, because smsc with smpp 3.3 version will be shocked on TLV. That's a wrong approach. Just think how to get such extra things in generic way and I'm the first who get my +1 vote... I have no clue at the moment how todo it (just not thought about it). > > A very simple way could be to add sections enclosed in comments > > /* This is for all protocols */ > > /* This is for SMPP only */ > > etc. > (possibly by using DEFINES if saving a few bytes is an issue) which defines? message is the generic struct that is also used for communication between bearerbox <-> XXXbox... > > The data in these sections will be referenced only in the relevant part of > Kannel code, so that will not clutter it up, if the design is kept modular > enough. > > - J�rgen Thomsen > > > > Summary: Implementation of User Message Reference > Description: > I have implemented the SMPP User Message Reference field i.e. an integer > which can be provided with the message to the SMSC. > Currently I am using it to tag the account number of my customers onto the > message, which is stored with other identifying data in the SMSC database, > so I can search for the messages for a specific customer through the > webinterface provided by the SMSC. > > J�rgen Thomsen > ======================================================================= > > ----------------------------------------------------------------------- > tolj - 12-09-04 13:55 GMT > ----------------------------------------------------------------------- > > For the given patch, I vote -1, which means refusing to commit. Reason: > The user message reference field is an 3.4 optional field that you > "simply" drop into the higher abstraction layer (the msg struct), and it > only applies for SMPP. This way it is easy for you to patch it, but it > "breaks" semantically the architecture of the code and the abstraction > layers depending on other SMSC modules. > > There is currently no way to "define" SMPP optional parameters on-the-fly, > and we don't want to mangle the optional fields into the SMSC abstracted > msg structure. -- Thanks, Alex
