Re: [Bacnet-developers] TSM ideas
Brought to you by:
skarg
|
From: Steve K. <st...@ka...> - 2010-12-13 15:51:34
|
Hi Peter, > I'm doing a bit of work with some client side code at the moment and had > ideas for some tweaks to the TSM code that might make handling responses > easier and allow for sending multiple requests out and matching up the > responses that come back simpler.. I assume you reviewed Julien's client side code in the branch to see if there are some useful nuggets in there. https://bacnet.svn.sourceforge.net/svnroot/bacnet/branches/jbennet/bacnet-stack-0-5-7/ > 1. Instead of having the acknowledge handlers decode responses as they > arrive I was thinking of adding storage for the complete response into the > TSM struct and having the client call the decoders when they find the > response has arrived. Alternativly the copy of the sent request could be > over written with the response, the (comments for some of the code suggest > that it be usefull to examine the sent packet when the response arrives but > how realistic is it to expect to decode it in those circumstances?). > Instead of being set to idle when the response arrives, the status would be > set to indicate "response received". The demo/handler/h_arf_a.c AtomicReadFileACK uses the TSM to figure out the file instance of a returned chunk of file. The ARF-A reqeust could be re-written to save the invoke ID and file instance locally before sending. > 2. Abort, Reject and Error responses would get stored in the TSM so that > they can be examined without relying on global variables. Again invoke id > status would be marked as "response received" and a flag could be examined > to see if it was a normal response or one of the error types. So maybe using a structure like this (with an IFDEF for segmentation or not): typedef struct bacnet_tsm_data { bool Allocated; BACNET_TSM_STATE State; uint8_t ActualWindowSize; uint32_t SequenceNumber; uint32_t NumSegments; uint8_t ProposedWindowSize; uint8_t SentAllSegments; uint8_t RetryCount; uint8_t SegmentRetryCount; uint8_t InitialSequenceNumber; uint8_t LastSequenceNumber; uint32_t SegmentTimer; uint32_t ApduTimer; uint8_t InvokeID; uint32_t RequestTimer; BACNET_ADDRESS ClientAddress; BACNET_ADDRESS ServerAddress; /* if we are answering the question */ bool AmServer; /* NPDU priority */ uint8_t Priority; /* the network layer info */ BACNET_NPDU_DATA NpduData; /* values retrieved from the original request */ bool SegmentedResponseAccepted; uint16_t MaxSegmentsAccepted; uint16_t MaxApduLengthAccepted; /* APDU size to use */ uint16_t SegmentLength; uint8_t *TransmitBuffers[MAX_WINDOW_SIZE]; uint8_t MessageBuffer[MAX_MESSAGE_LEN]; uint32_t MessageLength; uint8_t ServiceChoice; BOOL Confirmed; uint8_t SegmentAckBuffer[MAX_NPDU + 3]; BACNET_TSM_CALLBACK CallbackFunction; void *CallbackHandle; BACNET_ERROR_CLASS ErrorClass; BACNET_ERROR_CODE ErrorCode; uint32_t DroppedSegment; } BACNET_TSM_DATA; > 3. We don't matchup the invoke id and source address at the moment so there > is a chance that a delayed response from some device could trigger a "false > positive" if an invoke ID has been re used in the mean time. It is not VERY > likely but if it occurred it would be a difficult one to track down. > Implementing some of this in a way that can avoid breaking existing code if > the feature is not required may require some thinking so I thought I'd > solicit some opinions first. It would be good to rework our existing simplistic TSM module to fix that bug. I have been thinking for some time to change the TSM to be more central to the code - maybe for the 0.6 release (I hope to make a 0.5.8 release this week - I didn't finish it over Thanksgiving). Perhaps even have a small tsm-tiny.c and a tsm.c module. Some of the changes I am looking to do are shown here in a ReadProperty example (untested code): tsm = tsm_alloc(SERVICE_CONFIRMED_READ_PROPERTY); if (tsm) { tsm_npdu_set(src, dest, false, true, MESSAGE_PRIORITY_NORMAL); /* encode the APDU portion of the packet */ data.object_type = object_type; data.object_instance = object_instance; data.object_property = object_property; data.array_index = array_index; len = rp_encode_apdu( tsm_message_buffer(tsm), tsm_message_size(tsm), &data); if (len < 0) { tsm_free(tsm); return false; } tsm_message_length_set(tsm, len); tsm_send_request(tsm, CallbackFunction, CallbackHandle); } a. supply the buffer used in creating responses or requests (a tsm_alloc that is called instead of using the Handler_Transmit_Buffer global). b. use the tsm to send to the right datalink(s) (in the case of routing or multihome). c. change buffer handling of high level encode/decode functions to handle the length safely by adding an extra length parameter. This would keep the developer from needing duplicate or over-length buffers and actually have the code be safe from buffer overrun. Best Regards, Steve -- http://steve.kargs.net/ |