Mercurial > hg > orthanc

changeset 2335:174c3616ab6d

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
code review
author Sebastien Jodogne <s.jodogne@gmail.com>
date Mon, 17 Jul 2017 10:25:25 +0200
parents dd26536454a0
children a95beca72e99
files OrthancServer/DicomProtocol/DicomUserConnection.cpp OrthancServer/OrthancFindRequestHandler.cpp OrthancServer/ServerEnumerations.cpp
diffstat 3 files changed, 38 insertions(+), 17 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancServer/DicomProtocol/DicomUserConnection.cpp	Sat Jul 15 13:13:00 2017 +0200
+++ b/OrthancServer/DicomProtocol/DicomUserConnection.cpp	Mon Jul 17 10:25:25 2017 +0200
@@ -484,12 +484,13 @@
   static ParsedDicomFile* ConvertQueryFields(const DicomMap& fields,
                                              ModalityManufacturer manufacturer)
   {
-    // Fix outgoing C-Find requests
-    // issue for Syngo.Via and its solution was reported by
-    // Emsy Chan by private mail on 2015-06-17. According to
-    // Robert van Ommen (2015-11-30), the same fix is required for
-    // Agfa Impax.
-    // solutions was generalized for generic manufacturer since it seems to affect PhilipsADW, GEWAServer as well (check issue #31)
+    // Fix outgoing C-Find requests issue for Syngo.Via and its
+    // solution was reported by Emsy Chan by private mail on
+    // 2015-06-17. According to Robert van Ommen (2015-11-30), the
+    // same fix is required for Agfa Impax. This was generalized for
+    // generic manufacturer since it seems to affect PhilipsADW,
+    // GEWAServer as well:
+    // https://bitbucket.org/sjodogne/orthanc/issues/31/
 
     switch (manufacturer)
     {
@@ -503,9 +504,12 @@
 
         for (std::set<DicomTag>::const_iterator it = tags.begin(); it != tags.end(); ++it)
         {
-          // Replace a "*" query by an empty query ("") for "date" or "all" value representations depending on the manufacturer.
-          if ((manufacturer == ModalityManufacturer_GenericNoWildcards)
-              || (manufacturer == ModalityManufacturer_GenericNoWildcardInDates && FromDcmtkBridge::LookupValueRepresentation(*it) == ValueRepresentation_Date))
+          // Replace a "*" wildcard query by an empty query ("") for
+          // "date" or "all" value representations depending on the
+          // type of manufacturer.
+          if (manufacturer == ModalityManufacturer_GenericNoWildcards ||
+              (manufacturer == ModalityManufacturer_GenericNoWildcardInDates &&
+               FromDcmtkBridge::LookupValueRepresentation(*it) == ValueRepresentation_Date))
           {
             const DicomValue* value = fix->TestAndGetValue(*it);
 
--- a/OrthancServer/OrthancFindRequestHandler.cpp	Sat Jul 15 13:13:00 2017 +0200
+++ b/OrthancServer/OrthancFindRequestHandler.cpp	Mon Jul 17 10:25:25 2017 +0200
@@ -451,7 +451,9 @@
                                                  const DicomTag& tag,
                                                  ModalityManufacturer manufacturer)
   {
-    // wathever the manufacturer, remote the GenericGroupLength tags (http://dicom.nema.org/medical/dicom/current/output/chtml/part05/sect_7.2.html)
+    // Whatever the manufacturer, remove the GenericGroupLength tags
+    // http://dicom.nema.org/medical/dicom/current/output/chtml/part05/sect_7.2.html
+    // https://bitbucket.org/sjodogne/orthanc/issues/31/
     if (tag.GetGroup() == 0x0000)
     {
       return false;
--- a/OrthancServer/ServerEnumerations.cpp	Sat Jul 15 13:13:00 2017 +0200
+++ b/OrthancServer/ServerEnumerations.cpp	Mon Jul 17 10:25:25 2017 +0200
@@ -382,6 +382,9 @@
 
   ModalityManufacturer StringToModalityManufacturer(const std::string& manufacturer)
   {
+    ModalityManufacturer result;
+    bool obsolete = false;
+    
     if (manufacturer == "Generic")
     {
       return ModalityManufacturer_Generic;
@@ -410,21 +413,33 @@
     {
       return ModalityManufacturer_Vitrea;
     }
-    else if (manufacturer == "AgfaImpax" || manufacturer == "SyngoVia")
+    else if (manufacturer == "AgfaImpax" ||
+             manufacturer == "SyngoVia")
     {
-      LOG(WARNING) << "The " << manufacturer << " manufacturer is obsolete since Orthanc 1.2.1.  To guarantee compatibility with future Orthanc version, you should use \"GenericNoWildcardInDates\" instead in your configuration file.";
-      return ModalityManufacturer_GenericNoWildcardInDates;
+      result = ModalityManufacturer_GenericNoWildcardInDates;
+      obsolete = true;
     }
-    else if (manufacturer == "EFilm2" || manufacturer == "MedInria")
+    else if (manufacturer == "EFilm2" ||
+             manufacturer == "MedInria")
     {
-      LOG(WARNING) << "The " << manufacturer << " manufacturer is obsolete since Orthanc 1.2.1.  To guarantee compatibility with future Orthanc version, you should remove it from your configuration file.";
-
-      return ModalityManufacturer_Generic;
+      result = ModalityManufacturer_Generic;
+      obsolete = true;
     }
     else
     {
       throw OrthancException(ErrorCode_ParameterOutOfRange);
     }
+
+    if (obsolete)
+    {
+      LOG(WARNING) << "The \"" << manufacturer << "\" manufacturer is obsolete since "
+                   << "Orthanc 1.2.1. To guarantee compatibility with future Orthanc "
+                   << "releases, you should replace it by \""
+                   << EnumerationToString(result)
+                   << "\" in your configuration file.";
+    }
+
+    return result;
   }