From 95cf193601cc55b3538a36fa049815e06e9cc176 Mon Sep 17 00:00:00 2001
From: Florian Pose <fp@igh-essen.com>
Date: Thu, 24 Jul 2008 14:56:57 +0000
Subject: [PATCH] Bugfixes and improvements.

---
 TODO                     |  1 +
 tool/Command.cpp         | 24 ++++++++---------
 tool/Command.h           |  3 ++-
 tool/CommandAlias.cpp    |  3 ++-
 tool/CommandConfig.cpp   | 23 ++++++++---------
 tool/CommandData.cpp     |  6 ++---
 tool/CommandDomains.cpp  | 10 +++----
 tool/CommandDownload.cpp |  5 ++--
 tool/CommandPdos.cpp     | 11 ++++----
 tool/CommandSdos.cpp     |  6 ++---
 tool/CommandSlaves.cpp   | 20 +++++++-------
 tool/CommandStates.cpp   |  2 +-
 tool/CommandUpload.cpp   |  3 +--
 tool/CommandXml.cpp      |  6 ++---
 tool/main.cpp            | 56 ++++++++++++++++++++--------------------
 15 files changed, 90 insertions(+), 89 deletions(-)

diff --git a/TODO b/TODO
index 6c58adf8..93788af5 100644
--- a/TODO
+++ b/TODO
@@ -22,6 +22,7 @@ Version 1.4.0:
     - Display attached device's MAC address instead of ff's.
     - Data type abbreviations.
     - Add a -n (numeric) switch.
+    - Check for options, remove global variables.
 
 Future issues:
 
diff --git a/tool/Command.cpp b/tool/Command.cpp
index 90d3e0df..b9f0c5b2 100644
--- a/tool/Command.cpp
+++ b/tool/Command.cpp
@@ -53,29 +53,29 @@ bool Command::matchesAbbrev(const string &abb) const
     
 /*****************************************************************************/
 
-void Command::throwInvalidUsageException(const stringstream &s)
+string Command::numericInfo()
 {
-    throw InvalidUsageException(s);
+    stringstream str;
+
+    str << "Numerical values can be specified either with decimal (no" << endl
+        << "prefix), octal (prefix '0') or hexadecimal (prefix '0x') base."
+        << endl;
+
+    return str.str();
 }
 
 /*****************************************************************************/
 
-void Command::throwCommandException(const stringstream &s)
+void Command::throwInvalidUsageException(const stringstream &s)
 {
-    throw CommandException(s);
+    throw InvalidUsageException(s);
 }
 
 /*****************************************************************************/
 
-string Command::numericInfo()
+void Command::throwCommandException(const stringstream &s)
 {
-    stringstream str;
-
-    str << "Numerical values can be specified either with decimal (no" << endl
-        << "prefix), octal (prefix '0') or hexadecimal (prefix '0x') base."
-        << endl;
-
-    return str.str();
+    throw CommandException(s);
 }
 
 /****************************************************************************/
diff --git a/tool/Command.h b/tool/Command.h
index 14d420f7..dc362501 100644
--- a/tool/Command.h
+++ b/tool/Command.h
@@ -76,12 +76,13 @@ class Command
         typedef vector<string> StringVector;
         virtual void execute(MasterDevice &, const StringVector &) = 0;
 
+        static string numericInfo();
+
     protected:
         void throwInvalidUsageException(const stringstream &);
         void throwCommandException(const stringstream &);
 
 		enum {BreakAfterBytes = 16};
-        static string numericInfo();
 
     private:
 		string name;
diff --git a/tool/CommandAlias.cpp b/tool/CommandAlias.cpp
index da1131a5..7cd780bd 100644
--- a/tool/CommandAlias.cpp
+++ b/tool/CommandAlias.cpp
@@ -31,7 +31,8 @@ string CommandAlias::helpString() const
         << getBriefDescription() << endl
         << endl
         << "Arguments:" << endl
-        << "  ALIAS must be an unsigned 16 bit number. Zero means no alias."
+        << "  ALIAS must be an unsigned 16 bit number. Zero means" << endl
+        << "        removing an alias address." << endl
         << endl << endl
         << "Command-specific options:" << endl
         << "  --slave -s <index>  Positive numerical ring position, or 'all'"
diff --git a/tool/CommandConfig.cpp b/tool/CommandConfig.cpp
index d574fd9f..0e9025ee 100644
--- a/tool/CommandConfig.cpp
+++ b/tool/CommandConfig.cpp
@@ -26,10 +26,9 @@ string CommandConfig::helpString() const
 {
     stringstream str;
 
-    str << "[OPTIONS]" << endl
+    str << getName() << " [OPTIONS]" << endl
     	<< endl
-    	<< "Output information about the slave configurations" << endl
-    	<< "supplied by the application." << endl
+    	<< getBriefDescription() << endl
     	<< endl
     	<< "Without the --verbose option, slave configurations are" << endl
     	<< "output one-per-line. Example:" << endl
@@ -39,25 +38,25 @@ string CommandConfig::helpString() const
     	<< "|       |                      |  \\- Slave is operational."
 		<< endl
     	<< "|       |                      \\- Slave has been found." << endl
-    	<< "|       \\- Hexadecimal vendor ID and product code, separated"
-		<< endl
-    	<< "|          by a slash." << endl
-    	<< "\\- Decimal alias and position, separated by a colon." << endl
+    	<< "|       \\- Vendor ID and product code (both" << endl
+    	<< "|          hexadecimal)." << endl
+    	<< "\\- Alias and relative position (both decimal)." << endl
     	<< endl
     	<< "With the --verbose option given, the configured Pdos and" << endl
-    	<< "Sdos are additionally printed." << endl
+    	<< "Sdos are output in addition." << endl
     	<< endl
     	<< "Command-specific options:" << endl
-    	<< "  --verbose  -v  Show detailed configurations." << endl;
+    	<< "  --verbose -v  Show detailed configurations." << endl;
 
 	return str.str();
 }
 
 /*****************************************************************************/
 
-/*****************************************************************************/
-
-bool operator<(const ec_ioctl_config_t &a, const ec_ioctl_config_t &b)
+bool operator<(
+        const ec_ioctl_config_t &a,
+        const ec_ioctl_config_t &b
+        )
 {
     return a.alias < b.alias
         || (a.alias == b.alias && a.position < b.position);
diff --git a/tool/CommandData.cpp b/tool/CommandData.cpp
index d4446cc1..f7d80542 100644
--- a/tool/CommandData.cpp
+++ b/tool/CommandData.cpp
@@ -25,12 +25,12 @@ string CommandData::helpString() const
     str << getName() << " [OPTIONS]" << endl
     	<< endl
     	<< getBriefDescription() << endl
+        << endl
+        << "Data of multiple domains are concatenated." << endl
     	<< endl
     	<< "Command-specific options:" << endl
     	<< "  --domain -d <index>  Positive numerical domain index, or" << endl
-    	<< "                       'all' for all domains (default). In" << endl
-    	<< "                       this case, data of all domains are" << endl
-		<< "                       concatenated." << endl
+    	<< "                       'all' for all domains (default)." << endl
     	<< endl
 		<< numericInfo();
 
diff --git a/tool/CommandDomains.cpp b/tool/CommandDomains.cpp
index b880de59..ac6b6311 100644
--- a/tool/CommandDomains.cpp
+++ b/tool/CommandDomains.cpp
@@ -23,7 +23,7 @@ string CommandDomains::helpString() const
 {
     stringstream str;
 
-	str << getName() << " [OPTIONS]"
+	str << getName() << " [OPTIONS]" << endl
     	<< endl
     	<< getBriefDescription() << endl
         << endl
@@ -36,7 +36,7 @@ string CommandDomains::helpString() const
     	<< "(LRD/LWR/LRW) is displayed followed by the domain's" << endl
     	<< "process data size in byte. The last values are the current" << endl
     	<< "datagram working counter sum and the expected working" << endl
-    	<< "counter sum. If the values are equal, all Pdos are exchanged."
+    	<< "counter sum. If the values are equal, all Pdos were exchanged."
 		<< endl << endl
     	<< "If the --verbose option is given, the participating slave" << endl
     	<< "configurations/FMMUs and the current process data are" << endl
@@ -46,7 +46,7 @@ string CommandDomains::helpString() const
 		<< endl
     	<< "  SlaveConfig 1001:0, SM3 ( Input), LogAddr 0x00000006, Size 6"
 		<< endl
-    	<< "    00 00 00 00 00 00" << endl
+    	<< "    0x00 0x00 0x00 0x00 0x00 0x00" << endl
     	<< endl
     	<< "The process data are displayed as hexadecimal bytes." << endl
     	<< endl
@@ -55,7 +55,7 @@ string CommandDomains::helpString() const
     	<< "                        or 'all' for all domains (default)."
 		<< endl
     	<< "  --verbose -v          Show FMMUs and process data" << endl
-		<< "                        additionally." << endl
+		<< "                        in addition." << endl
     	<< endl
 		<< numericInfo();
 
@@ -143,7 +143,7 @@ void CommandDomains::showDomain(MasterDevice &m, unsigned int domainIndex)
         for (j = 0; j < fmmu.data_size; j++) {
             if (j && !(j % BreakAfterBytes))
                 cout << endl << "    ";
-            cout << setw(2)
+            cout << "0x" << setw(2)
                 << (unsigned int) *(processData + dataOffset + j) << " ";
         }
         cout << endl;
diff --git a/tool/CommandDownload.cpp b/tool/CommandDownload.cpp
index 9612c3fe..d7bf6088 100644
--- a/tool/CommandDownload.cpp
+++ b/tool/CommandDownload.cpp
@@ -38,7 +38,7 @@ string CommandDownload::helpString() const
     	<< "These are the valid Sdo entry data types:" << endl
     	<< "  int8, int16, int32, uint8, uint16, uint32, string." << endl
     	<< endl
-    	<< "Arguments:"
+    	<< "Arguments:" << endl
     	<< "  INDEX    is the Sdo index and must be an unsigned" << endl
 		<< "           16 bit number." << endl
     	<< "  SUBINDEX is the Sdo entry subindex and must be an" << endl
@@ -49,8 +49,7 @@ string CommandDownload::helpString() const
     	<< "Command-specific options:" << endl
     	<< "  --slave -s <index>  Positive numerical ring position" << endl
 		<< "                      (mandatory)." << endl
-    	<< "  --type  -t <type>   Forced Sdo entry data type (see" << endl
-		<< "                      above)." << endl
+    	<< "  --type  -t <type>   Sdo entry data type (see above)." << endl
     	<< endl
 		<< numericInfo();
 
diff --git a/tool/CommandPdos.cpp b/tool/CommandPdos.cpp
index 01d1cc6f..71384738 100644
--- a/tool/CommandPdos.cpp
+++ b/tool/CommandPdos.cpp
@@ -35,17 +35,18 @@ string CommandPdos::helpString() const
     	<< "   size (value from the SII), control register and enable" << endl
     	<< "   word. Example:" << endl
 		<< endl
-    	<< "   SM3: PhysAddr 0x1100, DefaultSize 0, ControlRegister 0x20,"
+    	<< "   SM3: PhysAddr 0x1100, DefaultSize 0, ControlRegister 0x20, "
 		<< "Enable 1" << endl
     	<< endl
     	<< "2) Assigned Pdos - Pdo direction, hexadecimal index and" << endl
-		<< "   -if available- the Pdo name. Example:" << endl
+		<< "   the Pdo name, if avaliable. Note that a 'Tx' and 'Rx'" << endl
+        << "   are seen from the slave's point of view. Example:" << endl
     	<< endl
     	<< "   TxPdo 0x1a00 \"Channel1\"" << endl
     	<< endl
     	<< "3) Mapped Pdo entries - Pdo entry index and subindex (both" << endl
-    	<< "   hexadecimal), the length in bit and -if available- the" << endl
-    	<< "   description. Example:" << endl
+    	<< "   hexadecimal), the length in bit and the description, if" << endl
+    	<< "   available. Example:" << endl
     	<< endl
     	<< "   Pdo entry 0x3101:01, 8 bit, \"Status\"" << endl
     	<< endl
@@ -55,7 +56,7 @@ string CommandPdos::helpString() const
     	<< endl
     	<< "Command-specific options:" << endl
     	<< "  --slave -s <index>  Positive numerical ring position," << endl
-    	<< "                      or 'all' forall slaves (default)." << endl
+    	<< "                      or 'all' for all slaves (default)." << endl
     	<< endl
 		<< numericInfo();
 
diff --git a/tool/CommandSdos.cpp b/tool/CommandSdos.cpp
index 1f80d556..7939f05a 100644
--- a/tool/CommandSdos.cpp
+++ b/tool/CommandSdos.cpp
@@ -41,12 +41,12 @@ string CommandSdos::helpString() const
     	<< endl
     	<< "   0x1018:01, uint32, 32 bit, \"Vendor id\"" << endl
     	<< endl
-    	<< "If the --quiet option is given, only the Sdos are printed."
+    	<< "If the --quiet option is given, only the Sdos are output."
 		<< endl << endl
     	<< "Command-specific options:" << endl
     	<< "  --slave -s <index>  Positive numerical ring position," << endl
-		<< "                      'all' for all slaves (default)." << endl
-    	<< "  --quiet -q          Print only Sdos (without Sdo" << endl
+		<< "                      or 'all' for all slaves (default)." << endl
+    	<< "  --quiet -q          Only output Sdos (without the Sdo" << endl
 		<< "                      entries)." << endl
     	<< endl
 		<< numericInfo();
diff --git a/tool/CommandSlaves.cpp b/tool/CommandSlaves.cpp
index f81ff8a0..8935948e 100644
--- a/tool/CommandSlaves.cpp
+++ b/tool/CommandSlaves.cpp
@@ -33,20 +33,20 @@ string CommandSlaves::helpString() const
         << endl
         << "1  5555:0  PREOP  +  EL3162 2C. Ana. Input 0-10V" << endl
         << "|  |    |  |      |  |" << endl
-        << "|  |    |  |      |  \\- Name from SII if avaliable," << endl
-        << "|  |    |  |      |     otherwise hexadecimal vendor ID" << endl
-        << "|  |    |  |      |     and product code separated by a" << endl
-        << "|  |    |  |      |     colon." << endl
+        << "|  |    |  |      |  \\- Name from the SII if avaliable," << endl
+        << "|  |    |  |      |     otherwise vendor ID and product" << endl
+        << "|  |    |  |      |     code (both hexadecimal)." << endl
         << "|  |    |  |      \\- Error flag. '+' means no error," << endl
-        << "|  |    |  |         'E' means that scanning or" << endl
+        << "|  |    |  |         'E' means that scan or" << endl
         << "|  |    |  |         configuration failed." << endl
-        << "|  |    |  \\- Current slave state." << endl
+        << "|  |    |  \\- Current application-layer state." << endl
         << "|  |    \\- Relative position (decimal) after the last" << endl
         << "|  |       slave with an alias address set." << endl
-        << "|  \\- Alias address of the slave (if set to non-zero)," << endl
-        << "|     or the alias of the last slave with an alias set," << endl
-        << "|     or zero if there is none." << endl
-        << "\\- Ring position (use this with any --slave option)." << endl
+        << "|  \\- Alias address of the slave (if set), or the alias" << endl
+        << "|     of the last slave with an alias, or zero if not" << endl
+        << "|     applicable" << endl
+        << "\\- Absolute ring position in the bus (use this with any" << endl
+        << "   --slave option)." << endl
         << endl
         << "If the --verbose option is given, a detailed (multi-line)" << endl
         << "description is output for each slave." << endl
diff --git a/tool/CommandStates.cpp b/tool/CommandStates.cpp
index b65de44d..b317951e 100644
--- a/tool/CommandStates.cpp
+++ b/tool/CommandStates.cpp
@@ -27,7 +27,7 @@ string CommandStates::helpString() const
         << getBriefDescription() << endl
         << endl
         << "Arguments:" << endl
-        << "  STATE can be 'INIT', 'PREOP', 'SAFEOP', or 'OP'" << endl
+        << "  STATE can be 'INIT', 'PREOP', 'SAFEOP', or 'OP'." << endl
         << endl
         << "Command-specific options:" << endl
         << "  --slave -s <index>  Positive numerical ring position," << endl
diff --git a/tool/CommandUpload.cpp b/tool/CommandUpload.cpp
index b96cca37..3b9c13af 100644
--- a/tool/CommandUpload.cpp
+++ b/tool/CommandUpload.cpp
@@ -47,8 +47,7 @@ string CommandUpload::helpString() const
         << "Command-specific options:" << endl
         << "  --slave -s <index>  Positive numerical ring position" << endl
         << "                      (mandatory)." << endl
-        << "  --type  -t <type>   Forced Sdo entry data type (see" << endl
-        << "                      above)." << endl
+        << "  --type  -t <type>   Sdo entry data type (see above)." << endl
         << endl
         << numericInfo();
 
diff --git a/tool/CommandXml.cpp b/tool/CommandXml.cpp
index 1175d00e..497188a7 100644
--- a/tool/CommandXml.cpp
+++ b/tool/CommandXml.cpp
@@ -23,14 +23,14 @@ string CommandXml::helpString() const
 {
     stringstream str;
 
-    str << getName() << " [OPTIONS]"
+    str << getName() << " [OPTIONS]" << endl
         << endl
         << getBriefDescription() << endl
         << endl
         << "Note that the Pdo information can either originate" << endl
         << "from the SII or from the CoE communication area. For" << endl
-        << "some slaves, this is dependant on the last slave" << endl
-        << "configuration." << endl
+        << "slaves, that support configuring Pdo assignment and" << endl
+        << "mapping, the output depends on the last configuration." << endl
         << endl
         << "Command-specific options:" << endl
         << "  --slave -s <index>  Positive numerical ring position," << endl
diff --git a/tool/main.cpp b/tool/main.cpp
index 6b062c7b..8d3fd6cf 100644
--- a/tool/main.cpp
+++ b/tool/main.cpp
@@ -49,8 +49,9 @@ bool helpRequested = false;
 
 /*****************************************************************************/
 
-void printUsage()
+string usage()
 {
+    stringstream str;
     CommandList::const_iterator ci;
     size_t maxWidth = 0;
 
@@ -60,19 +61,17 @@ void printUsage()
         }
     }
 
-    cerr
-        << "Usage: " << binaryBaseName << " <COMMAND> [OPTIONS] [ARGUMENTS]"
+    str << "Usage: " << binaryBaseName << " <COMMAND> [OPTIONS] [ARGUMENTS]"
         << endl << endl
 		<< "Commands (can be abbreviated):" << endl;
 
-    cerr << left;
+    str << left;
     for (ci = commandList.begin(); ci != commandList.end(); ci++) {
-        cerr << "  " << setw(maxWidth) << (*ci)->getName()
+        str << "  " << setw(maxWidth) << (*ci)->getName()
             << "  " << (*ci)->getBriefDescription() << endl;
     }
 
-    cerr
-        << endl
+    str << endl
 		<< "Global options:" << endl
         << "  --master  -m <master>  Index of the master to use. Default: 0."
 		<< endl
@@ -81,14 +80,14 @@ void printUsage()
         << "  --verbose -v           Output more information." << endl
         << "  --help    -h           Show this help." << endl
         << endl
-        << "Numerical values can be specified either with decimal "
-        << "(no prefix)," << endl
-        << "octal (prefix '0') or hexadecimal (prefix '0x') base." << endl
+        << Command::numericInfo()
         << endl
         << "Call '" << binaryBaseName
         << " <COMMAND> --help' for command-specific help." << endl
         << endl
         << "Send bug reports to " << PACKAGE_BUGREPORT << "." << endl;
+
+    return str.str();
 }
 
 /*****************************************************************************/
@@ -118,8 +117,8 @@ void getOptions(int argc, char **argv)
             case 'm':
                 number = strtoul(optarg, &remainder, 0);
                 if (remainder == optarg || *remainder || number < 0) {
-                    cerr << "Invalid master number " << optarg << "!" << endl;
-                    printUsage();
+                    cerr << "Invalid master number " << optarg << "!" << endl
+                        << endl << usage();
                     exit(1);
                 }
 				masterIndex = number;
@@ -132,9 +131,8 @@ void getOptions(int argc, char **argv)
                     number = strtoul(optarg, &remainder, 0);
                     if (remainder == optarg || *remainder
                             || number < 0 || number > 0xFFFF) {
-                        cerr << "Invalid slave position "
-                            << optarg << "!" << endl;
-                        printUsage();
+                        cerr << "Invalid slave position " << optarg << "!"
+                            << endl << endl << usage();
                         exit(1);
                     }
                     slavePosition = number;
@@ -148,8 +146,7 @@ void getOptions(int argc, char **argv)
                     number = strtoul(optarg, &remainder, 0);
                     if (remainder == optarg || *remainder || number < 0) {
                         cerr << "Invalid domain index "
-							<< optarg << "!" << endl;
-                        printUsage();
+							<< optarg << "!" << endl << endl << usage();
                         exit(1);
                     }
                     domainIndex = number;
@@ -177,7 +174,7 @@ void getOptions(int argc, char **argv)
                 break;
 
             case '?':
-                printUsage();
+                cerr << endl << usage();
                 exit(1);
 
             default:
@@ -189,11 +186,14 @@ void getOptions(int argc, char **argv)
 	argCount = argc - optind;
 
     if (!argCount) {
-        if (!helpRequested) {
-            cerr << "Please specify a command!" << endl;
+        if (helpRequested) {
+            cout << usage();
+            exit(0);
+        } else {
+            cerr << "Please specify a command!" << endl
+                << endl << usage();
+            exit(1);
         }
-        printUsage();
-        exit(!helpRequested);
 	}
 
     commandName = argv[optind];
@@ -264,10 +264,11 @@ int main(int argc, char **argv)
             cmd = matchingCommands.front();
             if (!helpRequested) {
                 try {
+                    cmd->setVerbosity(verbosity);
                     cmd->execute(masterDev, commandArgs);
                 } catch (InvalidUsageException &e) {
                     cerr << e.what() << endl << endl;
-                    cerr << cmd->helpString();
+                    cerr << binaryBaseName << " " << cmd->helpString();
                     retval = 1;
                 } catch (CommandException &e) {
                     cerr << e.what() << endl;
@@ -277,7 +278,7 @@ int main(int argc, char **argv)
                     retval = 1;
                 }
             } else {
-                cout << cmd->helpString();
+                cout << binaryBaseName << " " << cmd->helpString();
             }
         } else {
             cerr << "Ambiguous command abbreviation! Matching:" << endl;
@@ -286,13 +287,12 @@ int main(int argc, char **argv)
                     ci++) {
                 cerr << (*ci)->getName() << endl;
             }
-            cerr << endl;
-            printUsage();
+            cerr << endl << usage();
             retval = 1;
         }
     } else {
-        cerr << "Unknown command " << commandName << "!" << endl << endl;
-        printUsage();
+        cerr << "Unknown command " << commandName << "!" << endl
+            << endl << usage();
         retval = 1;
     }
 
-- 
GitLab