Merge branch 'develop' into bugfix/java-command-headers

CHANGELOG.md

@@ -4,6 +4,10 @@
 * :beetle: Fix keep-alive timer not properly calculated. See [#407](https://github.com/dnp3/opendnp3/pull/407).
 * :beetle: Fix `LinkContext` and `MContext` possible lifetime issue.
   See [#407](https://github.com/dnp3/opendnp3/pull/407).
+* :beetle: Fix UDP reconnect delay not being honoured. Also, initial UDP read errors
+  (due to ICMP packets) do not close the socket immediately. See [#438](https://github.com/dnp3/opendnp3/pull/438).
+* :coffin: Deprecate the `LinkConfig` constructor with an unused `useConfirms` argument.
+  See [#439](https://github.com/dnp3/opendnp3/pull/439).
 * :beetle: In Java, `CommandHeader` builder methods now all return `CommandHeaders`.
   See [#440](https://github.com/dnp3/opendnp3/pull/440).
 

cpp/examples/master-udp/main.cpp

@@ -39,8 +39,8 @@ int main(int argc, char* argv[])
     DNP3Manager manager(1, ConsoleLogger::Create());
 
     // Connect via a UDP socket to a outstation
-    auto channel = manager.AddUDPChannel("udpclient", logLevels, ChannelRetry::Default(), IPEndpoint("0.0.0.0", 20000),
-                                         IPEndpoint("192.168.0.106", 19999), PrintingChannelListener::Create());
+    auto channel = manager.AddUDPChannel("udpclient", logLevels, ChannelRetry::Default(), IPEndpoint("127.0.0.1", 20000),
+                                         IPEndpoint("127.0.0.1", 20001), PrintingChannelListener::Create());
 
     // The master config object for a master. The default are
     // useable, but understanding the options are important.

cpp/examples/outstation-udp/main.cpp

@@ -68,8 +68,8 @@ int main(int argc, char* argv[])
     DNP3Manager manager(1, ConsoleLogger::Create());
 
     // Create a UDP socket
-    auto channel = manager.AddUDPChannel("server", logLevels, ChannelRetry::Default(), IPEndpoint("192.168.0.106", 19999),
-                                         IPEndpoint("192.168.0.193", 20000), PrintingChannelListener::Create());
+    auto channel = manager.AddUDPChannel("udpserver", logLevels, ChannelRetry::Default(), IPEndpoint("127.0.0.1", 20001),
+                                         IPEndpoint("127.0.0.1", 20000), PrintingChannelListener::Create());
 
     // The main object for a outstation. The defaults are useable,
     // but understanding the options are important.

cpp/lib/CMakeLists.txt

@@ -25,7 +25,7 @@ set(opendnp3_public_headers
     ./include/opendnp3/app/MeasurementInfo.h
     ./include/opendnp3/app/MeasurementTypes.h
     ./include/opendnp3/app/OctetData.h
-    ./include/opendnp3/app/OctetString.h    
+    ./include/opendnp3/app/OctetString.h
 
     ./include/opendnp3/app/parsing/ICollection.h
 
@@ -70,7 +70,7 @@ set(opendnp3_public_headers
     ./include/opendnp3/gen/FunctionCode.h
     ./include/opendnp3/gen/GroupVariation.h
     ./include/opendnp3/gen/IndexQualifierMode.h
-    ./include/opendnp3/gen/IntervalUnits.h    
+    ./include/opendnp3/gen/IntervalUnits.h
     ./include/opendnp3/gen/LinkFunction.h
     ./include/opendnp3/gen/LinkStatus.h
     ./include/opendnp3/gen/MasterTaskType.h
@@ -80,7 +80,7 @@ set(opendnp3_public_headers
     ./include/opendnp3/gen/PointClass.h
     ./include/opendnp3/gen/QualifierCode.h
     ./include/opendnp3/gen/RestartMode.h
-    ./include/opendnp3/gen/RestartType.h    
+    ./include/opendnp3/gen/RestartType.h
     ./include/opendnp3/gen/ServerAcceptMode.h
     ./include/opendnp3/gen/StaticAnalogOutputStatusVariation.h
     ./include/opendnp3/gen/StaticAnalogVariation.h
@@ -161,7 +161,7 @@ set(opendnp3_public_headers
 	./include/opendnp3/util/StaticOnly.h
 	./include/opendnp3/util/TimeDuration.h
     ./include/opendnp3/util/Timestamp.h
-	./include/opendnp3/util/Uncopyable.h	
+	./include/opendnp3/util/Uncopyable.h
     ./include/opendnp3/util/UTCTimestamp.h
 )
 
@@ -210,7 +210,7 @@ set(opendnp3_private_headers
     ./src/app/parsing/Collections.h
     ./src/app/parsing/CountIndexParser.h
     ./src/app/parsing/CountParser.h
-    ./src/app/parsing/DNPTimeParsing.h    
+    ./src/app/parsing/DNPTimeParsing.h
     ./src/app/parsing/Functions.h
     ./src/app/parsing/IAPDUHandler.h
     ./src/app/parsing/IWhiteList.h
@@ -249,12 +249,12 @@ set(opendnp3_private_headers
     ./src/gen/FlagsTypeSerialization.h
     ./src/gen/FlowControlSerialization.h
     ./src/gen/FunctionCodeSerialization.h
-    ./src/gen/GroupVariationSerialization.h    
-    ./src/gen/IntervalUnitsSerialization.h    
+    ./src/gen/GroupVariationSerialization.h
+    ./src/gen/IntervalUnitsSerialization.h
     ./src/gen/LinkFunctionSerialization.h
     ./src/gen/ParitySerialization.h
     ./src/gen/QualifierCodeSerialization.h
-    ./src/gen/StopBitsSerialization.h    
+    ./src/gen/StopBitsSerialization.h
 
     ./src/gen/objects/Group1.h
     ./src/gen/objects/Group2.h
@@ -283,7 +283,7 @@ set(opendnp3_private_headers
     ./src/gen/objects/Group110.h
     ./src/gen/objects/Group111.h
     ./src/gen/objects/Group112.h
-    ./src/gen/objects/Group113.h    
+    ./src/gen/objects/Group113.h
 
     ./src/link/CRC.h
     ./src/link/IFrameSink.h
@@ -362,7 +362,7 @@ set(opendnp3_private_headers
     ./src/outstation/IEventReceiver.h
     ./src/outstation/IEventRecorder.h
     ./src/outstation/IEventSelector.h
-    ./src/outstation/IINHelpers.h    
+    ./src/outstation/IINHelpers.h
     ./src/outstation/IResponseLoader.h
     ./src/outstation/IStaticSelector.h
     ./src/outstation/OctetStringSerializer.h
@@ -434,13 +434,13 @@ set(opendnp3_src
 	./src/app/MeasurementInfo.cpp
 	./src/app/MeasurementTypes.cpp
     ./src/app/OctetData.cpp
-    ./src/app/QualityFlags.cpp    
+    ./src/app/QualityFlags.cpp
 
     ./src/app/parsing/APDUHeaderParser.cpp
     ./src/app/parsing/APDUParser.cpp
     ./src/app/parsing/BitReader.cpp
     ./src/app/parsing/CountIndexParser.cpp
-    ./src/app/parsing/CountParser.cpp    
+    ./src/app/parsing/CountParser.cpp
     ./src/app/parsing/IAPDUHandler.cpp
     ./src/app/parsing/NumParser.cpp
     ./src/app/parsing/ObjectHeaderParser.cpp
@@ -470,12 +470,12 @@ set(opendnp3_src
     ./src/gen/AnalogOutputStatusQuality.cpp
     ./src/gen/AnalogQuality.cpp
     ./src/gen/AssignClassType.cpp
-    ./src/gen/Attributes.cpp    
+    ./src/gen/Attributes.cpp
     ./src/gen/BinaryOutputStatusQuality.cpp
     ./src/gen/BinaryQuality.cpp
     ./src/gen/ChannelState.cpp
     ./src/gen/CommandPointState.cpp
-    ./src/gen/CommandStatus.cpp    
+    ./src/gen/CommandStatus.cpp
     ./src/gen/CounterQuality.cpp
     ./src/gen/DoubleBit.cpp
     ./src/gen/DoubleBitBinaryQuality.cpp
@@ -487,12 +487,12 @@ set(opendnp3_src
     ./src/gen/EventDoubleBinaryVariation.cpp
     ./src/gen/EventFrozenCounterVariation.cpp
     ./src/gen/EventMode.cpp
-    ./src/gen/EventOctetStringVariation.cpp    
+    ./src/gen/EventOctetStringVariation.cpp
     ./src/gen/FlagsType.cpp
     ./src/gen/FlowControl.cpp
     ./src/gen/FrozenCounterQuality.cpp
     ./src/gen/FunctionCode.cpp
-    ./src/gen/GroupVariation.cpp    
+    ./src/gen/GroupVariation.cpp
     ./src/gen/IndexQualifierMode.cpp
     ./src/gen/IntervalUnits.cpp
     ./src/gen/LinkFunction.cpp
@@ -504,7 +504,7 @@ set(opendnp3_src
     ./src/gen/PointClass.cpp
     ./src/gen/QualifierCode.cpp
     ./src/gen/RestartMode.cpp
-    ./src/gen/RestartType.cpp    
+    ./src/gen/RestartType.cpp
     ./src/gen/ServerAcceptMode.cpp
     ./src/gen/StaticAnalogOutputStatusVariation.cpp
     ./src/gen/StaticAnalogVariation.cpp
@@ -513,14 +513,14 @@ set(opendnp3_src
     ./src/gen/StaticCounterVariation.cpp
     ./src/gen/StaticDoubleBinaryVariation.cpp
     ./src/gen/StaticFrozenCounterVariation.cpp
-    ./src/gen/StaticOctetStringVariation.cpp    
+    ./src/gen/StaticOctetStringVariation.cpp
     ./src/gen/StaticTimeAndIntervalVariation.cpp
     ./src/gen/StaticTypeBitmask.cpp
     ./src/gen/StopBits.cpp
     ./src/gen/TaskCompletion.cpp
     ./src/gen/TimestampQuality.cpp
     ./src/gen/TimeSyncMode.cpp
-    ./src/gen/TripCloseCode.cpp    
+    ./src/gen/TripCloseCode.cpp
 
     ./src/gen/objects/Group1.cpp
     ./src/gen/objects/Group2.cpp
@@ -542,7 +542,7 @@ set(opendnp3_src
     ./src/gen/objects/Group43.cpp
     ./src/gen/objects/Group50.cpp
     ./src/gen/objects/Group51.cpp
-    ./src/gen/objects/Group52.cpp    
+    ./src/gen/objects/Group52.cpp
 
     ./src/link/Addresses.cpp
     ./src/link/CRC.cpp
@@ -613,9 +613,9 @@ set(opendnp3_src
     ./src/outstation/OutstationStates.cpp
     ./src/outstation/ReadHandler.cpp
     ./src/outstation/RequestHistory.cpp
-    ./src/outstation/ResponseContext.cpp    
+    ./src/outstation/ResponseContext.cpp
     ./src/outstation/SimpleCommandHandler.cpp
-    ./src/outstation/StaticDataMap.cpp    
+    ./src/outstation/StaticDataMap.cpp
     ./src/outstation/StaticWriters.cpp
     ./src/outstation/UpdateBuilder.cpp
     ./src/outstation/WriteHandler.cpp

cpp/lib/include/opendnp3/link/LinkConfig.h

@@ -36,7 +36,6 @@ struct LinkConfig
     LinkConfig(
         bool isMaster, uint16_t localAddr, uint16_t remoteAddr, TimeDuration timeout, TimeDuration keepAliveTimeout)
         :
-
           IsMaster(isMaster),
           LocalAddr(localAddr),
           RemoteAddr(remoteAddr),
@@ -45,9 +44,19 @@ struct LinkConfig
     {
     }
 
+    [[deprecated("Use LinkConfig(bool) instead.")]]
     LinkConfig(bool isMaster, bool useConfirms)
         :
+          IsMaster(isMaster),
+          LocalAddr(isMaster ? 1 : 1024),
+          RemoteAddr(isMaster ? 1024 : 1),
+          Timeout(TimeDuration::Seconds(1)),
+          KeepAliveTimeout(TimeDuration::Minutes(1))
+    {
+    }
 
+    LinkConfig(bool isMaster)
+        :
           IsMaster(isMaster),
           LocalAddr(isMaster ? 1 : 1024),
           RemoteAddr(isMaster ? 1024 : 1),

cpp/lib/include/opendnp3/master/MasterStackConfig.h

@@ -31,7 +31,7 @@ namespace opendnp3
 */
 struct MasterStackConfig
 {
-    MasterStackConfig() : link(true, false) {}
+    MasterStackConfig() : link(true) {}
 
     /// Master config
     MasterParams master;

cpp/lib/include/opendnp3/outstation/OutstationStackConfig.h

@@ -34,9 +34,9 @@ namespace opendnp3
 */
 struct OutstationStackConfig
 {
-    OutstationStackConfig() : link(false, false) {}
+    OutstationStackConfig() : link(false) {}
 
-    OutstationStackConfig(const DatabaseConfig& database) : database(database), link(false, false) {}
+    OutstationStackConfig(const DatabaseConfig& database) : database(database), link(false) {}
 
     // Configuration of the database
     DatabaseConfig database;

cpp/lib/src/channel/IOHandler.cpp

@@ -52,7 +52,7 @@ void IOHandler::OnReadComplete(const std::error_code& ec, size_t num)
 {
     if (ec)
     {
-        SIMPLE_LOG_BLOCK(this->logger, flags::WARN, ec.message().c_str());
+        FORMAT_LOG_BLOCK(this->logger, flags::WARN, "read error: %s", ec.message().c_str());
 
         this->Reset();
 
@@ -73,7 +73,7 @@ void IOHandler::OnWriteComplete(const std::error_code& ec, size_t num)
 
     if (ec)
     {
-        SIMPLE_LOG_BLOCK(this->logger, flags::WARN, ec.message().c_str());
+        FORMAT_LOG_BLOCK(this->logger, flags::WARN, "write error: %s", ec.message().c_str());
         this->Reset();
 
         this->UpdateListener(ChannelState::OPENING);

cpp/lib/src/channel/UDPClientIOHandler.cpp

@@ -59,7 +59,9 @@ void UDPClientIOHandler::SuspendChannelAccept()
 
 void UDPClientIOHandler::OnChannelShutdown()
 {
-    this->BeginChannelAccept();
+    this->retrytimer = this->executor->start(this->retry.reconnectDelay.value, [this, self = shared_from_this()]() {
+        this->BeginChannelAccept();
+    });
 }
 
 bool UDPClientIOHandler::TryOpen(const TimeDuration& delay)
@@ -89,19 +91,19 @@ bool UDPClientIOHandler::TryOpen(const TimeDuration& delay)
         else
         {
             FORMAT_LOG_BLOCK(this->logger, flags::INFO, "UDP socket binded to: %s, port %u, sending to %s, port %u",
-                             localEndpoint.address.c_str(), localEndpoint.port, remoteEndpoint.address.c_str(),
-                             remoteEndpoint.port);
+                             socket.local_endpoint().address().to_string().c_str(), socket.local_endpoint().port(),
+                             socket.remote_endpoint().address().to_string().c_str(), socket.remote_endpoint().port());
 
             if (client)
             {
-                this->OnNewChannel(UDPSocketChannel::Create(executor, std::move(socket)));
+                this->OnNewChannel(UDPSocketChannel::Create(executor, this->logger, std::move(socket)));
             }
         }
     };
 
     FORMAT_LOG_BLOCK(this->logger, flags::INFO, "Binding UDP socket to: %s, port %u, resolving address: %s, port %u",
-                     localEndpoint.address.c_str(), localEndpoint.port, remoteEndpoint.address.c_str(),
-                     remoteEndpoint.port);
+                     localEndpoint.address.c_str(), localEndpoint.port,
+                     remoteEndpoint.address.c_str(), remoteEndpoint.port);
 
     this->client->Open(localEndpoint, remoteEndpoint, cb);
 

cpp/lib/src/channel/UDPSocketChannel.cpp

@@ -20,18 +20,46 @@
 
 #include "channel/UDPSocketChannel.h"
 
+#include "logging/LogMacros.h"
+
+static constexpr uint8_t MAX_FIRST_READ_RETRIES = 5;
+
 namespace opendnp3
 {
 
 UDPSocketChannel::UDPSocketChannel(const std::shared_ptr<exe4cpp::StrandExecutor>& executor,
-                                   asio::ip::udp::socket socket)
-    : IAsyncChannel(executor), socket(std::move(socket))
+                                   const Logger& logger, asio::ip::udp::socket socket)
+    : IAsyncChannel(executor), logger(logger), socket(std::move(socket)), first_successful_read(false), num_first_read_retries(0)
 {
 }
 
 void UDPSocketChannel::BeginReadImpl(ser4cpp::wseq_t dest)
 {
-    auto callback = [this](const std::error_code& ec, size_t num) { this->OnReadCallback(ec, num); };
+    auto callback = [this](const std::error_code& ec, size_t num) {
+        if(!this->first_successful_read && this->num_first_read_retries < MAX_FIRST_READ_RETRIES)
+        {
+            if(ec)
+            {
+                FORMAT_LOG_BLOCK(this->logger, flags::WARN, "read error: %s", ec.message().c_str());
+                FORMAT_LOG_BLOCK(this->logger, flags::DBG, "UDP ignoring initial errors (%d of %d)", this->num_first_read_retries + 1, MAX_FIRST_READ_RETRIES);
+
+                // We ignore failed reads until we get a successful one
+                const auto no_ec = std::error_code{};
+                this->OnReadCallback(no_ec, num);
+
+                // Avoid infinite loop
+                ++this->num_first_read_retries;
+
+                return;
+            }
+            else
+            {
+                this->first_successful_read = true;
+            }
+        }
+
+        this->OnReadCallback(ec, num);
+    };
 
     socket.async_receive(asio::buffer(dest, dest.length()), this->executor->wrap(callback));
 }

cpp/lib/src/channel/UDPSocketChannel.h

@@ -21,6 +21,7 @@
 #define OPENDNP3_UDPSOCKETCHANNEL_H
 
 #include "channel/IAsyncChannel.h"
+#include "opendnp3/logging/Logger.h"
 
 namespace opendnp3
 {
@@ -30,20 +31,23 @@ class UDPSocketChannel final : public IAsyncChannel
 
 public:
     static std::shared_ptr<IAsyncChannel> Create(std::shared_ptr<exe4cpp::StrandExecutor> executor,
-                                                 asio::ip::udp::socket socket)
+                                                 const Logger& logger, asio::ip::udp::socket socket)
     {
-        return std::make_shared<UDPSocketChannel>(executor, std::move(socket));
+        return std::make_shared<UDPSocketChannel>(executor, logger, std::move(socket));
     }
 
-    UDPSocketChannel(const std::shared_ptr<exe4cpp::StrandExecutor>& executor, asio::ip::udp::socket socket);
+    UDPSocketChannel(const std::shared_ptr<exe4cpp::StrandExecutor>& executor, const Logger& logger, asio::ip::udp::socket socket);
 
 protected:
     void BeginReadImpl(ser4cpp::wseq_t dest) final;
     void BeginWriteImpl(const ser4cpp::rseq_t& buffer) final;
     void ShutdownImpl() final;
 
 private:
+    Logger logger;
     asio::ip::udp::socket socket;
+    bool first_successful_read;
+    uint8_t num_first_read_retries;
 };
 
 } // namespace opendnp3