Repository: pydicom/pynetdicom

Issue:
Can't send move requests to Siemens network nodes
**Is your feature request related to a problem? Please describe.**

It appears that many (most? all?) Siemens instruments have some buggy behavior in how they handle the "MoveDestination" inside a move request. If the AE Title provided in this field is padded with more white space than strictly needed to make the length even (e.g. an AE Title that is 9 chars is padded to be 16 chars instead of just 10) then the move request will be denied. This prevents the use of pynetdicom to send move requests to Siemens instruments.

Conversely, the actual association request PDU must have the AE Title padded to the full 16 chars in order to be accepted.

I guess there may be other situations where an AE Title needs to be passed in as an attribute and similar restrictions apply.

**Describe the solution you'd like**

The dcmtk library seems to only pad the AE Title out to 16 bytes in the association request, and otherwise just pad to the next even length.




Discussion:

Patch:
diff --git a/docs/changelog/v1.5.0.rst b/docs/changelog/v1.5.0.rst
index 8bd8344df..5c88acc2d 100644
--- a/docs/changelog/v1.5.0.rst
+++ b/docs/changelog/v1.5.0.rst
@@ -10,6 +10,14 @@ Fixes
   closing its sockets (:issue:`382`)
 * Improved the robustness of determining the local IP address (:issue:`394`)
 
+Enhancements
+............
+
+* Added configuration option :attr:`_config.USE_SHORT_DIMSE_AET
+  <pynetdicom._config.USE_SHORT_DIMSE_AET>` so elements with a VR of *AE* in
+  DIMSE messages are only padded to the shortest even length rather than to
+  the maximum 16 characters. (:issue:`403`)
+
 Changes
 .......
 
diff --git a/docs/reference/config.rst b/docs/reference/config.rst
index 6cb9323fa..6e43fb58b 100644
--- a/docs/reference/config.rst
+++ b/docs/reference/config.rst
@@ -10,3 +10,4 @@ Configuration Options (:mod:`pynetdicom._config`)
 
    ENFORCE_UID_CONFORMANCE
    LOG_HANDLER_LEVEL
+   USE_SHORT_DIMSE_AET
diff --git a/pynetdicom/_config.py b/pynetdicom/_config.py
index e115d9f9a..53093bc9d 100644
--- a/pynetdicom/_config.py
+++ b/pynetdicom/_config.py
@@ -31,3 +31,19 @@ Examples
 >>> from pynetdicom import _config
 >>> _config.ENFORCE_UID_CONFORMANCE = True
 """
+
+
+USE_SHORT_DIMSE_AET = False
+"""Use short AE titles in DIMSE messages.
+
+If ``False`` then elements with a VR of AE in DIMSE messages will be padded
+with trailing spaces up to the maximum allowable length (16 bytes), otherwise
+they will be padded with zero or one space to the smallest possible even
+length (i.e an AE title with 7 characters will be trailing padded to 8).
+
+Examples
+--------
+
+>>> from pynetdicom import _config
+>>> _config.USE_SHORT_DIMSE_AET = True
+"""
diff --git a/pynetdicom/dimse_primitives.py b/pynetdicom/dimse_primitives.py
index 0b276f203..3d8371844 100644
--- a/pynetdicom/dimse_primitives.py
+++ b/pynetdicom/dimse_primitives.py
@@ -17,6 +17,7 @@ import logging
 from pydicom.tag import Tag
 from pydicom.uid import UID
 
+from pynetdicom import _config
 from pynetdicom.utils import validate_ae_title, validate_uid
 
 
@@ -589,7 +590,7 @@ class C_STORE(DIMSEPrimitive):
         if value:
             try:
                 self._move_originator_application_entity_title = (
-                    validate_ae_title(value)
+                    validate_ae_title(value, _config.USE_SHORT_DIMSE_AET)
                 )
             except ValueError:
                 LOGGER.error(
@@ -1154,7 +1155,9 @@ class C_MOVE(DIMSEPrimitive):
             value = codecs.encode(value, 'ascii')
 
         if value is not None:
-            self._move_destination = validate_ae_title(value)
+            self._move_destination = validate_ae_title(
+                value, _config.USE_SHORT_DIMSE_AET
+            )
         else:
             self._move_destination = None
 
diff --git a/pynetdicom/utils.py b/pynetdicom/utils.py
index fc9bfbc0a..0b5f6f5ae 100644
--- a/pynetdicom/utils.py
+++ b/pynetdicom/utils.py
@@ -71,7 +71,7 @@ def pretty_bytes(bytestream, prefix='  ', delimiter='  ', items_per_line=16,
     return lines
 
 
-def validate_ae_title(ae_title):
+def validate_ae_title(ae_title, use_short=False):
     """Return a valid AE title from `ae_title`, if possible.
 
     An AE title:
@@ -93,6 +93,10 @@ def validate_ae_title(ae_title):
     ----------
     ae_title : bytes
         The AE title to check.
+    use_short : bool, optional
+        If ``False`` (default) then pad AE titles with trailing spaces up to
+        the maximum allowable length (16 bytes), otherwise only pad odd length
+        AE titles with a single trailing space to make it even length.
 
     Returns
     -------
@@ -131,8 +135,12 @@ def validate_ae_title(ae_title):
 
     # Truncate if longer than 16 characters
     ae_title = ae_title[:16]
-    # Pad out to 16 characters using spaces
-    ae_title = ae_title.ljust(16)
+    if not use_short:
+        # Pad out to 16 characters using spaces
+        ae_title = ae_title.ljust(16)
+    elif len(ae_title) % 2:
+        # Pad to even length
+        ae_title += ' '
 
     # Unicode category: 'Cc' is control characters
     invalid = [


Test patch:
diff --git a/pynetdicom/tests/test_dimse_c.py b/pynetdicom/tests/test_dimse_c.py
index 678d7e280..19c622887 100644
--- a/pynetdicom/tests/test_dimse_c.py
+++ b/pynetdicom/tests/test_dimse_c.py
@@ -50,9 +50,11 @@ class TestPrimitive_C_STORE(object):
     """Test DIMSE C-STORE operations."""
     def setup(self):
         self.default_conformance = _config.ENFORCE_UID_CONFORMANCE
+        self.default_aet_length = _config.USE_SHORT_DIMSE_AET
 
     def teardown(self):
         _config.ENFORCE_UID_CONFORMANCE = self.default_conformance
+        _config.USE_SHORT_DIMSE_AET = self.default_aet_length
 
     def test_assignment(self):
         """ Check assignment works correctly """
@@ -320,6 +322,38 @@ class TestPrimitive_C_STORE(object):
         primitive.Status = 0x0000
         assert primitive.is_valid_response
 
+    def test_aet_short_false(self):
+        """Test using long AE titles."""
+        primitive = C_STORE()
+
+        _config.USE_SHORT_DIMSE_AET = False
+
+        primitive.MoveOriginatorApplicationEntityTitle = b'A'
+        aet = primitive.MoveOriginatorApplicationEntityTitle
+        assert b'A               ' == aet
+
+    def test_aet_short_true(self):
+        """Test using short AE titles."""
+        primitive = C_STORE()
+
+        _config.USE_SHORT_DIMSE_AET = True
+
+        primitive.MoveOriginatorApplicationEntityTitle = b'A'
+        aet = primitive.MoveOriginatorApplicationEntityTitle
+        assert b'A ' == aet
+
+        primitive.MoveOriginatorApplicationEntityTitle = b'ABCDEFGHIJKLMNO'
+        aet = primitive.MoveOriginatorApplicationEntityTitle
+        assert b'ABCDEFGHIJKLMNO ' == aet
+
+        primitive.MoveOriginatorApplicationEntityTitle = b'ABCDEFGHIJKLMNOP'
+        aet = primitive.MoveOriginatorApplicationEntityTitle
+        assert b'ABCDEFGHIJKLMNOP' == aet
+
+        primitive.MoveOriginatorApplicationEntityTitle = b'ABCDEFGHIJKLMNOPQ'
+        aet = primitive.MoveOriginatorApplicationEntityTitle
+        assert b'ABCDEFGHIJKLMNOP' == aet
+
 
 class TestPrimitive_C_FIND(object):
     """Test DIMSE C-FIND operations."""
@@ -789,9 +823,11 @@ class TestPrimitive_C_MOVE(object):
     """Test DIMSE C-MOVE operations."""
     def setup(self):
         self.default_conformance = _config.ENFORCE_UID_CONFORMANCE
+        self.default_aet_length = _config.USE_SHORT_DIMSE_AET
 
     def teardown(self):
         _config.ENFORCE_UID_CONFORMANCE = self.default_conformance
+        _config.USE_SHORT_DIMSE_AET = self.default_aet_length
 
     def test_assignment(self):
         """ Check assignment works correctly """
@@ -1047,6 +1083,34 @@ class TestPrimitive_C_MOVE(object):
         primitive.Status = 0x0000
         assert primitive.is_valid_response
 
+    def test_aet_short_false(self):
+        """Test using long AE titles."""
+        primitive = C_MOVE()
+
+        _config.USE_SHORT_DIMSE_AET = False
+
+        primitive.MoveDestination = b'A'
+        assert b'A               ' == primitive.MoveDestination
+
+    def test_aet_short_true(self):
+        """Test using short AE titles."""
+        primitive = C_MOVE()
+
+        _config.USE_SHORT_DIMSE_AET = True
+
+        primitive.MoveDestination = b'A'
+        aet = primitive.MoveDestination
+        assert b'A ' == primitive.MoveDestination
+
+        primitive.MoveDestination = b'ABCDEFGHIJKLMNO'
+        assert b'ABCDEFGHIJKLMNO ' == primitive.MoveDestination
+
+        primitive.MoveDestination = b'ABCDEFGHIJKLMNOP'
+        assert b'ABCDEFGHIJKLMNOP' == primitive.MoveDestination
+
+        primitive.MoveDestination = b'ABCDEFGHIJKLMNOPQ'
+        assert b'ABCDEFGHIJKLMNOP' == primitive.MoveDestination
+
 
 class TestPrimitive_C_ECHO(object):
     """Test DIMSE C-ECHO operations."""