search

JoelBender / bacpypes

BACpypes provides a BACnet application layer and network layer written in Python for daemons, scripting, and graphical interfaces.
MIT License
297 stars 129 forks source link

AtomicReadFileRequest returns nothing if file data is larger than max apdu size #73

Closed JoelBender closed 8 years ago

JoelBender commented 8 years ago

In appservice.py remoteDevice.segmentationSupported is not initialized properly. The get_device_info() function should be smarter than simply returning a generic DeviceInfo instance. That should at least reflect the information contain in the APCI of the request, namely the apduSA (segmented response accepted), apduMaxSegs (maximum segments accepted) and apduMaxResp (max response accepted).

This issue was found during an investigation of executing an AtomicReadFileRequest but I suspect that it will present itself for other cases of segmented requests and/or responses.

See thread in the mailing list archvies

KunalSaini commented 8 years ago

Hi , We are running into this issue when doing file transfer of size larger then the apduMaxSegs. Our target device supports 'segmented' apdu however since the code uses default value of 'noSegmentation', we are facing some issues. Can you refer to any workaround for now or do you have any branch you are working on for this issue?

JoelBender commented 8 years ago

No, I haven't started working on this problem yet. I'll post here when the issue branch is created. If you have some more details about the file transfer (stream or records, record sizes, file size, etc.) and you have a pcap file you could send me joel{at}carrickbender.com I would appreciate it. I could use that to create a set of tests.

KunalSaini commented 8 years ago

Thanks @JoelBender , on some further digging, I found out following issues

  1. when reading segment ack from the wire, the conversion fails.
  2. after receiving first segment ack the code fails to send subsequent request messages.
  3. as mentioned by you the default behavior is 'no segmentation'

for 1 and 2 , I tried these change - https://github.com/JoelBender/bacpypes/pull/83 and for 3 I have changed to 'segmentationBoth' in https://github.com/JoelBender/bacpypes/pull/84 .

Do share your thoughts.

JoelBender commented 8 years ago

Version 0.14.0 was just released that in theory will fix these problems. I would have liked to include unit tests that poke harder at the combinations of client-side and server-side state machine transitions. I'm going to close this issue, please open a new one if necessary.