From 8523fbed95bc2de1fba411c8ff12dcb426ed7aca Mon Sep 17 00:00:00 2001 From: jadamcrain Date: Mon, 27 Nov 2017 07:52:49 -0500 Subject: [PATCH 1/7] Add missing database configuration for octet strings --- cpp/libs/include/asiodnp3/DatabaseConfig.h | 5 ++++- cpp/libs/include/asiopal/Executor.h | 6 +++--- cpp/libs/src/asiodnp3/LinkSession.h | 14 +++++++------- cpp/libs/src/asiodnp3/OutstationStack.cpp | 1 + cpp/libs/src/asiodnp3/PrintingCommandCallback.cpp | 6 +++--- 5 files changed, 18 insertions(+), 14 deletions(-) diff --git a/cpp/libs/include/asiodnp3/DatabaseConfig.h b/cpp/libs/include/asiodnp3/DatabaseConfig.h index 7d6ccea802..b752845f6d 100644 --- a/cpp/libs/include/asiodnp3/DatabaseConfig.h +++ b/cpp/libs/include/asiodnp3/DatabaseConfig.h @@ -43,7 +43,8 @@ class DatabaseConfig frozenCounter(sizes.numFrozenCounter), boStatus(sizes.numBinaryOutputStatus), aoStatus(sizes.numAnalogOutputStatus), - timeAndInterval(sizes.numTimeAndInterval) + timeAndInterval(sizes.numTimeAndInterval), + octetString(sizes.numOctetString) { InitIndices(binary); InitIndices(doubleBinary); @@ -53,6 +54,7 @@ class DatabaseConfig InitIndices(boStatus); InitIndices(aoStatus); InitIndices(timeAndInterval); + InitIndices(octetString); } const opendnp3::DatabaseSizes sizes; @@ -65,6 +67,7 @@ class DatabaseConfig openpal::Array boStatus; openpal::Array aoStatus; openpal::Array timeAndInterval; + openpal::Array octetString; private: diff --git a/cpp/libs/include/asiopal/Executor.h b/cpp/libs/include/asiopal/Executor.h index ec725bcf5f..39f01dbcea 100644 --- a/cpp/libs/include/asiopal/Executor.h +++ b/cpp/libs/include/asiopal/Executor.h @@ -40,9 +40,9 @@ namespace asiopal * */ class Executor final : - public openpal::IExecutor, - public std::enable_shared_from_this, - private openpal::Uncopyable + public openpal::IExecutor, + public std::enable_shared_from_this, + private openpal::Uncopyable { public: diff --git a/cpp/libs/src/asiodnp3/LinkSession.h b/cpp/libs/src/asiodnp3/LinkSession.h index 4aafa16ea1..14b487ef67 100644 --- a/cpp/libs/src/asiodnp3/LinkSession.h +++ b/cpp/libs/src/asiodnp3/LinkSession.h @@ -40,13 +40,13 @@ namespace asiodnp3 { class LinkSession final : - public opendnp3::ILinkTx, - public asiopal::IChannelCallbacks, - private opendnp3::IFrameSink, - public std::enable_shared_from_this, - public asiopal::IResource, - private ISessionAcceptor, - private openpal::Uncopyable + public opendnp3::ILinkTx, + public asiopal::IChannelCallbacks, + private opendnp3::IFrameSink, + public std::enable_shared_from_this, + public asiopal::IResource, + private ISessionAcceptor, + private openpal::Uncopyable { public: diff --git a/cpp/libs/src/asiodnp3/OutstationStack.cpp b/cpp/libs/src/asiodnp3/OutstationStack.cpp index d0beef88c5..46bd3510d5 100644 --- a/cpp/libs/src/asiodnp3/OutstationStack.cpp +++ b/cpp/libs/src/asiodnp3/OutstationStack.cpp @@ -61,6 +61,7 @@ OutstationStack::OutstationStack( assign(config.dbConfig.boStatus, view.binaryOutputStatii); assign(config.dbConfig.aoStatus, view.analogOutputStatii); assign(config.dbConfig.timeAndInterval, view.timeAndIntervals); + assign(config.dbConfig.octetString, view.octetStrings); } diff --git a/cpp/libs/src/asiodnp3/PrintingCommandCallback.cpp b/cpp/libs/src/asiodnp3/PrintingCommandCallback.cpp index 7d3497df57..79df75af5e 100644 --- a/cpp/libs/src/asiodnp3/PrintingCommandCallback.cpp +++ b/cpp/libs/src/asiodnp3/PrintingCommandCallback.cpp @@ -36,9 +36,9 @@ opendnp3::CommandCallbackT PrintingCommandCallback::Get() { std::cout << "Header: " << res.headerIndex - << " Index: " << res.index - << " State: " << CommandPointStateToString(res.state) - << " Status: " << CommandStatusToString(res.status); + << " Index: " << res.index + << " State: " << CommandPointStateToString(res.state) + << " Status: " << CommandStatusToString(res.status); }; result.ForeachItem(print); }; From 343789da79fa0bc290e31340935edfff015e0fb4 Mon Sep 17 00:00:00 2001 From: jadamcrain Date: Mon, 27 Nov 2017 08:00:09 -0500 Subject: [PATCH 2/7] move tests for discontiguous indices to their own test suite --- cpp/tests/opendnp3/src/TestOutstation.cpp | 127 -------------- .../TestOutstationDiscontiguousIndices.cpp | 162 ++++++++++++++++++ 2 files changed, 162 insertions(+), 127 deletions(-) create mode 100644 cpp/tests/opendnp3/src/TestOutstationDiscontiguousIndices.cpp diff --git a/cpp/tests/opendnp3/src/TestOutstation.cpp b/cpp/tests/opendnp3/src/TestOutstation.cpp index 1492ebdd7b..da7567d0cd 100644 --- a/cpp/tests/opendnp3/src/TestOutstation.cpp +++ b/cpp/tests/opendnp3/src/TestOutstation.cpp @@ -516,133 +516,6 @@ TEST_CASE(SUITE("ReadByRangeHeader")) REQUIRE(t.lower->PopWriteAsHex() == "C2 81 80 00 1E 02 00 05 06 01 2A 00 01 29 00"); } -TEST_CASE(SUITE("ContiguousIndexesInDiscontiguousModeIntegrityScan")) -{ - // this will tell the outstation to use discontiguous index mode, but we won't change the address assignments - OutstationConfig config; - config.params.indexMode = IndexMode::Discontiguous; - OutstationTestObject t(config, DatabaseSizes::BinaryOnly(2)); - t.LowerLayerUp(); - - t.SendToOutstation(hex::IntegrityPoll(0)); - - REQUIRE(t.lower->PopWriteAsHex() == "C0 81 80 00 01 02 00 00 01 02 02"); -} - -TEST_CASE(SUITE("can safely update non-existent index in discontiguous index mode")) -{ - OutstationConfig config; - config.params.indexMode = IndexMode::Discontiguous; - OutstationTestObject t(config, DatabaseSizes::BinaryOnly(2)); - - auto view = t.context.GetConfigView(); - for (int i = 0; i < 2; ++i) - { - view.binaries[i].config.clazz = PointClass::Class1; - view.binaries[i].config.svariation = StaticBinaryVariation::Group1Var2; - view.binaries[i].config.evariation = EventBinaryVariation::Group2Var3; - view.binaries[i].config.vIndex = i + 2; - } - - // now send an update to boundary points that doesn't exist - REQUIRE_FALSE(t.context.GetUpdateHandler().Update(Binary(true), 0, EventMode::Suppress)); - REQUIRE_FALSE(t.context.GetUpdateHandler().Update(Binary(true), 1, EventMode::Suppress)); - // 2 & 3 exist - REQUIRE_FALSE(t.context.GetUpdateHandler().Update(Binary(true), 4, EventMode::Suppress)); -} - -TEST_CASE(SUITE("ContiguousIndexesInDiscontiguousModeRangeScan")) -{ - // this will tell the outstation to use discontiguous index mode, but we won't change the address assignments - OutstationConfig config; - config.params.indexMode = IndexMode::Discontiguous; - OutstationTestObject t(config, DatabaseSizes::BinaryOnly(2)); - t.LowerLayerUp(); - - t.SendToOutstation("C0 01 01 02 00 00 01"); - - REQUIRE(t.lower->PopWriteAsHex() == "C0 81 80 00 01 02 00 00 01 02 02"); -} - -std::string QueryDiscontiguousBinary(const std::string& request) -{ - OutstationConfig config; - config.params.indexMode = IndexMode::Discontiguous; - - OutstationTestObject t(config, DatabaseSizes::BinaryOnly(3)); - - // assign virtual indices to the database specified above - auto view = t.context.GetConfigView(); - view.binaries[0].config.vIndex = 2; - view.binaries[1].config.vIndex = 4; - view.binaries[2].config.vIndex = 5; - - t.LowerLayerUp(); - - t.Transaction([](IUpdateHandler & db) - { - db.Update(Binary(true, 0x01), 2, EventMode::Suppress); - db.Update(Binary(false, 0x01), 4, EventMode::Suppress); - }); - - t.SendToOutstation(request); - return t.lower->PopWriteAsHex(); -} - -TEST_CASE(SUITE("ReadDiscontiguousClass0")) -{ - REQUIRE(QueryDiscontiguousBinary("C0 01 3C 01 06") == "C0 81 80 00 01 02 00 02 02 81 01 02 00 04 05 01 02"); -} - -TEST_CASE(SUITE("ReadDiscontiguousBadRangeBelow")) -{ - // read 01 var 2, [00 : 01] - REQUIRE(QueryDiscontiguousBinary("C0 01 01 02 00 00 01") == "C0 81 80 04"); -} - -TEST_CASE(SUITE("ReadDiscontiguousBadRangeAbove")) -{ - // read 01 var 2, [06 : 09] - REQUIRE(QueryDiscontiguousBinary("C0 01 01 02 00 06 09") == "C0 81 80 04"); -} - - -TEST_CASE(SUITE("ReadDiscontiguousSingleRange")) -{ - // read 01 var 2, [02 : 02] - REQUIRE(QueryDiscontiguousBinary("C0 01 01 02 00 02 02") == "C0 81 80 00 01 02 00 02 02 81"); -} - -TEST_CASE(SUITE("ReadDiscontiguousDoubleRange")) -{ - // read 01 var 2, [04 : 05] - REQUIRE(QueryDiscontiguousBinary("C0 01 01 02 00 04 05") == "C0 81 80 00 01 02 00 04 05 01 02"); -} - -TEST_CASE(SUITE("ReadDiscontiguousPastUpperBound")) -{ - // read 01 var 2, [05 : 06] - REQUIRE(QueryDiscontiguousBinary("C0 01 01 02 00 05 06") == "C0 81 80 04 01 02 00 05 05 02"); -} - -TEST_CASE(SUITE("ReadDiscontiguousAllDataWithMultipleRanges")) -{ - // read 01 var 2, [02 : 02]; read 01 var 2, [04 : 05] - REQUIRE(QueryDiscontiguousBinary("C0 01 01 02 00 02 02 01 02 00 04 05") == "C0 81 80 00 01 02 00 02 02 81 01 02 00 04 05 01 02"); -} - -TEST_CASE(SUITE("ReadDiscontiguousAllDataWithMultipleRangesAndRangeError")) -{ - // read 01 var 2, [02 : 03]; read 01 var 2, [04 : 05] - REQUIRE(QueryDiscontiguousBinary("C0 01 01 02 00 02 03 01 02 00 04 05") == "C0 81 80 04 01 02 00 02 02 81 01 02 00 04 05 01 02"); -} - -TEST_CASE(SUITE("ReadDiscontiguousAllDataWithRangeError")) -{ - // read 01 var 2, [02 : 05] - REQUIRE(QueryDiscontiguousBinary("C0 01 01 02 00 02 05") == "C0 81 80 04 01 02 00 02 02 81 01 02 00 04 05 01 02"); -} - template void TestStaticType(const OutstationConfig& config, const DatabaseSizes& tmp, PointType value, const std::string& rsp, const std::function& configure) { diff --git a/cpp/tests/opendnp3/src/TestOutstationDiscontiguousIndices.cpp b/cpp/tests/opendnp3/src/TestOutstationDiscontiguousIndices.cpp new file mode 100644 index 0000000000..543d7cba2b --- /dev/null +++ b/cpp/tests/opendnp3/src/TestOutstationDiscontiguousIndices.cpp @@ -0,0 +1,162 @@ +/* + * Licensed to Green Energy Corp (www.greenenergycorp.com) under one or + * more contributor license agreements. See the NOTICE file distributed + * with this work for additional information regarding copyright ownership. + * Green Energy Corp licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except in + * compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * This project was forked on 01/01/2013 by Automatak, LLC and modifications + * may have been made to this file. Automatak, LLC licenses these modifications + * to you under the terms of the License. + */ +#include + +#include "mocks/OutstationTestObject.h" + +#include +#include + +using namespace std; +using namespace opendnp3; +using namespace openpal; +using namespace testlib; + +#define SUITE(name) "OutstationDiscontiguousIndexTestSuite - " name + + +TEST_CASE(SUITE("integrity scan returns expected result with default index assignments")) +{ + // this will tell the outstation to use discontiguous index mode, but we won't change the address assignments + OutstationConfig config; + config.params.indexMode = IndexMode::Discontiguous; + OutstationTestObject t(config, DatabaseSizes::BinaryOnly(2)); + t.LowerLayerUp(); + + t.SendToOutstation(hex::IntegrityPoll(0)); + + REQUIRE(t.lower->PopWriteAsHex() == "C0 81 80 00 01 02 00 00 01 02 02"); +} + +TEST_CASE(SUITE("can safely update non-existent index in discontiguous index mode")) +{ + OutstationConfig config; + config.params.indexMode = IndexMode::Discontiguous; + OutstationTestObject t(config, DatabaseSizes::BinaryOnly(2)); + + auto view = t.context.GetConfigView(); + for (int i = 0; i < 2; ++i) + { + view.binaries[i].config.clazz = PointClass::Class1; + view.binaries[i].config.svariation = StaticBinaryVariation::Group1Var2; + view.binaries[i].config.evariation = EventBinaryVariation::Group2Var3; + view.binaries[i].config.vIndex = i + 2; + } + + // now send an update to boundary points that doesn't exist + REQUIRE_FALSE(t.context.GetUpdateHandler().Update(Binary(true), 0, EventMode::Suppress)); + REQUIRE_FALSE(t.context.GetUpdateHandler().Update(Binary(true), 1, EventMode::Suppress)); + // 2 & 3 exist + REQUIRE_FALSE(t.context.GetUpdateHandler().Update(Binary(true), 4, EventMode::Suppress)); +} + +TEST_CASE(SUITE("range scan returns expected results with default index assignments")) +{ + // this will tell the outstation to use discontiguous index mode, but we won't change the address assignments + OutstationConfig config; + config.params.indexMode = IndexMode::Discontiguous; + OutstationTestObject t(config, DatabaseSizes::BinaryOnly(2)); + t.LowerLayerUp(); + + t.SendToOutstation("C0 01 01 02 00 00 01"); + + REQUIRE(t.lower->PopWriteAsHex() == "C0 81 80 00 01 02 00 00 01 02 02"); +} + +std::string QueryDiscontiguousBinary(const std::string& request) +{ + OutstationConfig config; + config.params.indexMode = IndexMode::Discontiguous; + + OutstationTestObject t(config, DatabaseSizes::BinaryOnly(3)); + + // assign virtual indices to the database specified above + auto view = t.context.GetConfigView(); + view.binaries[0].config.vIndex = 2; + view.binaries[1].config.vIndex = 4; + view.binaries[2].config.vIndex = 5; + + t.LowerLayerUp(); + + t.Transaction([](IUpdateHandler & db) + { + db.Update(Binary(true, 0x01), 2, EventMode::Suppress); + db.Update(Binary(false, 0x01), 4, EventMode::Suppress); + }); + + t.SendToOutstation(request); + return t.lower->PopWriteAsHex(); +} + +TEST_CASE(SUITE("ReadDiscontiguousClass0")) +{ + REQUIRE(QueryDiscontiguousBinary("C0 01 3C 01 06") == "C0 81 80 00 01 02 00 02 02 81 01 02 00 04 05 01 02"); +} + +TEST_CASE(SUITE("ReadDiscontiguousBadRangeBelow")) +{ + // read 01 var 2, [00 : 01] + REQUIRE(QueryDiscontiguousBinary("C0 01 01 02 00 00 01") == "C0 81 80 04"); +} + +TEST_CASE(SUITE("ReadDiscontiguousBadRangeAbove")) +{ + // read 01 var 2, [06 : 09] + REQUIRE(QueryDiscontiguousBinary("C0 01 01 02 00 06 09") == "C0 81 80 04"); +} + + +TEST_CASE(SUITE("ReadDiscontiguousSingleRange")) +{ + // read 01 var 2, [02 : 02] + REQUIRE(QueryDiscontiguousBinary("C0 01 01 02 00 02 02") == "C0 81 80 00 01 02 00 02 02 81"); +} + +TEST_CASE(SUITE("ReadDiscontiguousDoubleRange")) +{ + // read 01 var 2, [04 : 05] + REQUIRE(QueryDiscontiguousBinary("C0 01 01 02 00 04 05") == "C0 81 80 00 01 02 00 04 05 01 02"); +} + +TEST_CASE(SUITE("ReadDiscontiguousPastUpperBound")) +{ + // read 01 var 2, [05 : 06] + REQUIRE(QueryDiscontiguousBinary("C0 01 01 02 00 05 06") == "C0 81 80 04 01 02 00 05 05 02"); +} + +TEST_CASE(SUITE("ReadDiscontiguousAllDataWithMultipleRanges")) +{ + // read 01 var 2, [02 : 02]; read 01 var 2, [04 : 05] + REQUIRE(QueryDiscontiguousBinary("C0 01 01 02 00 02 02 01 02 00 04 05") == "C0 81 80 00 01 02 00 02 02 81 01 02 00 04 05 01 02"); +} + +TEST_CASE(SUITE("ReadDiscontiguousAllDataWithMultipleRangesAndRangeError")) +{ + // read 01 var 2, [02 : 03]; read 01 var 2, [04 : 05] + REQUIRE(QueryDiscontiguousBinary("C0 01 01 02 00 02 03 01 02 00 04 05") == "C0 81 80 04 01 02 00 02 02 81 01 02 00 04 05 01 02"); +} + +TEST_CASE(SUITE("ReadDiscontiguousAllDataWithRangeError")) +{ + // read 01 var 2, [02 : 05] + REQUIRE(QueryDiscontiguousBinary("C0 01 01 02 00 02 05") == "C0 81 80 04 01 02 00 02 02 81 01 02 00 04 05 01 02"); +} + From d1270d13cb7f6122ba66987689fd5db4693c058a Mon Sep 17 00:00:00 2001 From: jadamcrain Date: Mon, 27 Nov 2017 08:35:51 -0500 Subject: [PATCH 3/7] fix for using octet strings w/ discontiguous indices --- .../src/opendnp3/outstation/DatabaseBuffers.h | 46 ++++++++----------- .../src/opendnp3/outstation/StaticWriters.cpp | 41 +++++++++++------ .../TestOutstationDiscontiguousIndices.cpp | 19 ++++++++ 3 files changed, 67 insertions(+), 39 deletions(-) diff --git a/cpp/libs/src/opendnp3/outstation/DatabaseBuffers.h b/cpp/libs/src/opendnp3/outstation/DatabaseBuffers.h index a4491d4043..1505d5e1fb 100644 --- a/cpp/libs/src/opendnp3/outstation/DatabaseBuffers.h +++ b/cpp/libs/src/opendnp3/outstation/DatabaseBuffers.h @@ -227,39 +227,33 @@ template bool DatabaseBuffers::LoadType(HeaderWriter& writer) { auto range = ranges.Get(); - if (range.IsValid()) - { - auto view = buffers.GetArrayView(); + if (!range.IsValid()) return true; // no data to load - bool spaceRemaining = true; + auto view = buffers.GetArrayView(); - // ... load values, manipulate the range - while (spaceRemaining && range.IsValid()) + bool spaceRemaining = true; + + // ... load values, manipulate the range + while (spaceRemaining && range.IsValid()) + { + if (view[range.start].selection.selected) { - if (view[range.start].selection.selected) - { - /// lookup the specific write function based on the reporting variation - auto writeFun = StaticWriters::Get(view[range.start].selection.variation); + /// lookup the specific write function based on the reporting variation + auto writeFun = StaticWriters::Get(view[range.start].selection.variation); - // start writing a header, the invoked function will advance the range appropriately - spaceRemaining = writeFun(view, writer, range); - } - else - { - // just skip over values that are not selected - range.Advance(); - } + // start writing a header, the invoked function will advance the range appropriately + spaceRemaining = writeFun(view, writer, range); } + else + { + // just skip over values that are not selected + range.Advance(); + } + } - ranges.Set(range); + ranges.Set(range); - return spaceRemaining; - } - else - { - // no data to load - return true; - } + return spaceRemaining; } template diff --git a/cpp/libs/src/opendnp3/outstation/StaticWriters.cpp b/cpp/libs/src/opendnp3/outstation/StaticWriters.cpp index 7734830f13..fd6d2c6465 100644 --- a/cpp/libs/src/opendnp3/outstation/StaticWriters.cpp +++ b/cpp/libs/src/opendnp3/outstation/StaticWriters.cpp @@ -232,41 +232,56 @@ StaticWrite::func_t StaticWriters::Get(StaticTimeAndInterva template uint16_t WriteSomeOctetString(openpal::ArrayView, uint16_t>& view, Iterator& iterator, Range& range, uint8_t size) { + const Cell& start = view[range.start]; + uint16_t nextIndex = start.config.vIndex; + uint16_t num_written = 0; - while (range.IsValid()) + + while ( + range.IsValid() && + view[range.start].selection.value.Size() == size && + view[range.start].selection.selected && + (view[range.start].selection.variation == start.selection.variation) && + (view[range.start].config.vIndex == nextIndex) + ) { - if (view[range.start].selection.value.Size() != size) + if (iterator.Write(view[range.start].selection.value)) { - return num_written; + // deselect the value and advance the range + view[range.start].selection.selected = false; + ++num_written; + ++nextIndex; + range.Advance(); } - - if (!iterator.Write(view[range.start].selection.value)) + else { - return num_written; // not enough space + return false; } - - ++num_written; - range.Advance(); } + return num_written; } bool StaticWriters::Write(openpal::ArrayView, uint16_t>& view, HeaderWriter& writer, Range& range) { - while (range.IsValid()) + auto start = view[range.start].config.vIndex; + auto stop = view[range.stop].config.vIndex; + auto mapped = Range::From(start, stop); + + if (mapped.IsValid()) { const uint8_t sizeStartingSize = view[range.start].selection.value.Size(); const OctetStringSerializer serializer(false, sizeStartingSize); - if (range.IsOneByte()) + if (mapped.IsOneByte()) { - auto iter = writer.IterateOverRange(QualifierCode::UINT8_START_STOP, serializer, static_cast(range.start)); + auto iter = writer.IterateOverRange(QualifierCode::UINT8_START_STOP, serializer, static_cast(mapped.start)); const uint16_t num_written = WriteSomeOctetString(view, iter, range, sizeStartingSize); if (num_written == 0) return false; } else { - auto iter = writer.IterateOverRange(QualifierCode::UINT16_START_STOP, serializer, range.start); + auto iter = writer.IterateOverRange(QualifierCode::UINT16_START_STOP, serializer, mapped.start); const uint16_t num_written = WriteSomeOctetString(view, iter, range, sizeStartingSize); if (num_written == 0) return false; } diff --git a/cpp/tests/opendnp3/src/TestOutstationDiscontiguousIndices.cpp b/cpp/tests/opendnp3/src/TestOutstationDiscontiguousIndices.cpp index 543d7cba2b..f8aa0a8501 100644 --- a/cpp/tests/opendnp3/src/TestOutstationDiscontiguousIndices.cpp +++ b/cpp/tests/opendnp3/src/TestOutstationDiscontiguousIndices.cpp @@ -106,6 +106,25 @@ std::string QueryDiscontiguousBinary(const std::string& request) return t.lower->PopWriteAsHex(); } +TEST_CASE(SUITE("octet strings behave as expected with discontiguous indices")) +{ + // this will tell the outstation to use discontiguous index mode, but we won't change the address assignments + OutstationConfig config; + config.eventBufferConfig.maxOctetStringEvents = 2; + config.params.indexMode = IndexMode::Discontiguous; + OutstationTestObject t(config, DatabaseSizes::OctetStringOnly(2)); + { + auto view = t.context.GetConfigView(); + view.octetStrings[0].config.vIndex = 1; + view.octetStrings[1].config.vIndex = 3; + } + t.LowerLayerUp(); + + t.SendToOutstation(hex::IntegrityPoll(0)); + + REQUIRE(t.lower->PopWriteAsHex() == "C0 81 80 00 6E 01 00 01 01 00 6E 01 00 03 03 00"); +} + TEST_CASE(SUITE("ReadDiscontiguousClass0")) { REQUIRE(QueryDiscontiguousBinary("C0 01 3C 01 06") == "C0 81 80 00 01 02 00 02 02 81 01 02 00 04 05 01 02"); From 468ceebd9bd4b3d0e12514b7b941adca6f8fb89e Mon Sep 17 00:00:00 2001 From: jadamcrain Date: Mon, 27 Nov 2017 08:46:15 -0500 Subject: [PATCH 4/7] apply index conversion for .NET octet strings --- dotnet/bindings/CLRAdapter/src/Conversions.cpp | 1 + dotnet/bindings/CLRAdapter/src/Conversions.h | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/dotnet/bindings/CLRAdapter/src/Conversions.cpp b/dotnet/bindings/CLRAdapter/src/Conversions.cpp index 9311059c6c..e470911211 100644 --- a/dotnet/bindings/CLRAdapter/src/Conversions.cpp +++ b/dotnet/bindings/CLRAdapter/src/Conversions.cpp @@ -449,6 +449,7 @@ namespace Automatak ConvertEventConfig(lhs->binaryOutputStatii, rhs.boStatus); ConvertDeadbandConfig(lhs->analogOutputStatii, rhs.aoStatus); ConvertStaticConfig(lhs->timeAndIntervals, rhs.timeAndInterval); + ConvertIndexConfig(lhs->octetStrings, rhs.octetString); } opendnp3::GroupVariationID Conversions::Convert(PointClass clazz) diff --git a/dotnet/bindings/CLRAdapter/src/Conversions.h b/dotnet/bindings/CLRAdapter/src/Conversions.h index 7954252737..8ec9f3a2b9 100644 --- a/dotnet/bindings/CLRAdapter/src/Conversions.h +++ b/dotnet/bindings/CLRAdapter/src/Conversions.h @@ -178,11 +178,21 @@ namespace Automatak static void ApplyConfig(DatabaseTemplate^ lhs, asiodnp3::DatabaseConfig& rhs); template - static void ConvertStaticConfig(Source^ source, Target& target) + static void ConvertIndexConfig(Source^ source, Target& target) { for (int i = 0; i < source->Count; ++i) { - target[i].vIndex = source[i]->index; + target[i].vIndex = source[i]->index; + } + } + + template + static void ConvertStaticConfig(Source^ source, Target& target) + { + ConvertIndexConfig(source, target); + + for (int i = 0; i < source->Count; ++i) + { target[i].svariation = (typename Info::static_variation_t) source[i]->staticVariation; } } From fdf0ac5e2f2fcca1f36112683513f6280a447086 Mon Sep 17 00:00:00 2001 From: jadamcrain Date: Tue, 28 Nov 2017 12:12:17 -0500 Subject: [PATCH 5/7] OctetData::Set() returns false if input to copy is empty --- cpp/libs/src/opendnp3/app/OctetData.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cpp/libs/src/opendnp3/app/OctetData.cpp b/cpp/libs/src/opendnp3/app/OctetData.cpp index 0f34d3e15d..8a28e9ee19 100644 --- a/cpp/libs/src/opendnp3/app/OctetData.cpp +++ b/cpp/libs/src/opendnp3/app/OctetData.cpp @@ -42,11 +42,12 @@ OctetData::OctetData(const RSlice& input) : bool OctetData::Set(const openpal::RSlice& input) { - if (input.Size() > MAX_SIZE) return false; + if ((input.Size() < 1) || (input.Size() > MAX_SIZE)) return false; else { auto dest = buffer.GetWSlice(); input.CopyTo(dest); + this->size = static_cast(input.Size()); return true; } } From a3704f8b764b2f9753032e61eb3baa8f569935bf Mon Sep 17 00:00:00 2001 From: jadamcrain Date: Tue, 28 Nov 2017 12:59:46 -0500 Subject: [PATCH 6/7] Add overloaded Set() method and constructor to allow OctetString to be initialized using c-style strings --- cpp/libs/include/opendnp3/app/OctetData.h | 49 ++++++++++++++++++++- cpp/libs/include/opendnp3/app/OctetString.h | 25 +++++++++++ cpp/libs/src/opendnp3/app/OctetData.cpp | 32 ++++++++++++-- 3 files changed, 101 insertions(+), 5 deletions(-) diff --git a/cpp/libs/include/opendnp3/app/OctetData.h b/cpp/libs/include/opendnp3/app/OctetData.h index 734c316784..50c04b1292 100644 --- a/cpp/libs/include/opendnp3/app/OctetData.h +++ b/cpp/libs/include/opendnp3/app/OctetData.h @@ -38,7 +38,31 @@ class OctetData const static uint8_t MAX_SIZE = 255; + /** + * Construct with default length of 1 + */ OctetData(); + + /** + * Construct from a c-style string + * + * strlen() is used internally to determine the length + * + * If the length is 0, it is defaulted to 1 + * If the length is > 255, the first 255 bytes are copied. + * + * The null terminator is NOT copied as part of buffer + */ + OctetData(const char* input); + + /** + * Construct from read-only buffer slice + * + * + * If the length is 0, it is defaulted to 1 + * If the length is > 255, the first 255 bytes are copied. + * + */ OctetData(const openpal::RSlice& input); inline uint8_t Size() const @@ -46,12 +70,33 @@ class OctetData return size; } - bool Set(const openpal::RSlice&); - + /** + * Set the buffer to the input buffer/length if the input has length in the interval [1,255] + * + * @param input the input data to copy into this object + * + * @return true if the input meets the length requirements, false otherwise + */ + bool Set(const openpal::RSlice& input); + + /** + * Set the buffer equal to the supplied c-string if the input string has length in the interval [1,255] + * + * @param input c-style string to copy into this object + * + * @return true if the input meets the length requirements, false otherwise + */ + bool Set(const char* input); + + /** + * @return a view of the current data as a read-only slice + */ openpal::RSlice ToRSlice() const; private: + static openpal::RSlice ToSlice(const char* input); + openpal::StaticBuffer buffer; uint8_t size; }; diff --git a/cpp/libs/include/opendnp3/app/OctetString.h b/cpp/libs/include/opendnp3/app/OctetString.h index cfd3aacf35..00474efa10 100644 --- a/cpp/libs/include/opendnp3/app/OctetString.h +++ b/cpp/libs/include/opendnp3/app/OctetString.h @@ -33,12 +33,37 @@ class OctetString : public OctetData { public: + /** + * Construct with default length of 1 + */ OctetString() : OctetData() {} + /** + * Copy construct from another OctetString + */ OctetString(const OctetString& data) : OctetData(data) {} + /** + * Construct from a c-style string + * + * strlen() is used internally to determine the length + * + * If the length is 0, it is defaulted to 1 + * If the length is > 255, the first 255 bytes are copied. + * + * The null terminator is NOT copied as part of buffer + */ + OctetString(const char* input) : OctetData(input) + {} + + /** + * Construct from read-only buffer slice + * + * If the length is 0, it is defaulted to 1 + * If the length is > 255, the first 255 bytes are copied. + */ OctetString(const openpal::RSlice& buffer) : OctetData(buffer) {} diff --git a/cpp/libs/src/opendnp3/app/OctetData.cpp b/cpp/libs/src/opendnp3/app/OctetData.cpp index 8a28e9ee19..da55585b78 100644 --- a/cpp/libs/src/opendnp3/app/OctetData.cpp +++ b/cpp/libs/src/opendnp3/app/OctetData.cpp @@ -22,6 +22,9 @@ #include #include +#include + +#include using namespace openpal; @@ -33,11 +36,19 @@ OctetData::OctetData() : size(1) } +OctetData::OctetData(const char* input) : OctetData(ToSlice(input)) +{ + +} + OctetData::OctetData(const RSlice& input) : - size(openpal::Min(MAX_SIZE, input.Size())) + size(input.IsEmpty() ? 1 : openpal::Min(MAX_SIZE, input.Size())) { - auto dest = buffer.GetWSlice(); - input.Take(size).CopyTo(dest); + if (input.IsNotEmpty()) + { + auto dest = buffer.GetWSlice(); + input.Take(size).CopyTo(dest); + } } bool OctetData::Set(const openpal::RSlice& input) @@ -52,11 +63,26 @@ bool OctetData::Set(const openpal::RSlice& input) } } + +bool OctetData::Set(const char* input) +{ + const size_t length = strlen(input); + if (length > MAX_SIZE) return false; + return this->Set(openpal::RSlice(reinterpret_cast(input), static_cast(length))); +} + openpal::RSlice OctetData::ToRSlice() const { return buffer.ToRSlice(size); } +openpal::RSlice OctetData::ToSlice(const char* input) +{ + const size_t length = strlen(input); + if (length == 0) return openpal::RSlice::Empty(); + return openpal::RSlice(reinterpret_cast(input), length > MAX_SIZE ? MAX_SIZE : static_cast(length)); +} + } From 38ccd4bf60ff945351ad80f61d5343d7c674b8d4 Mon Sep 17 00:00:00 2001 From: jadamcrain Date: Thu, 30 Nov 2017 13:15:59 -0500 Subject: [PATCH 7/7] OctetData::Set() now behaves like constructors --- cpp/libs/include/opendnp3/app/OctetData.h | 21 ++++++++++++++------- cpp/libs/include/opendnp3/app/OctetString.h | 10 +++++----- cpp/libs/src/opendnp3/app/OctetData.cpp | 21 +++++++++++++-------- 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/cpp/libs/include/opendnp3/app/OctetData.h b/cpp/libs/include/opendnp3/app/OctetData.h index 50c04b1292..64e352079a 100644 --- a/cpp/libs/include/opendnp3/app/OctetData.h +++ b/cpp/libs/include/opendnp3/app/OctetData.h @@ -39,7 +39,7 @@ class OctetData const static uint8_t MAX_SIZE = 255; /** - * Construct with default length of 1 + * Construct with a default value of [0x00] (length == 1) */ OctetData(); @@ -48,8 +48,8 @@ class OctetData * * strlen() is used internally to determine the length * - * If the length is 0, it is defaulted to 1 - * If the length is > 255, the first 255 bytes are copied. + * If the length is 0, the default value of [0x00] is assigned + * If the length is > 255, only the first 255 bytes are copied. * * The null terminator is NOT copied as part of buffer */ @@ -59,9 +59,10 @@ class OctetData * Construct from read-only buffer slice * * - * If the length is 0, it is defaulted to 1 - * If the length is > 255, the first 255 bytes are copied. + * If the length is 0, the default value of [0x00] is assigned + * If the length is > 255, only the first 255 bytes are copied. * + * The null terminator is NOT copied as part of buffer */ OctetData(const openpal::RSlice& input); @@ -71,7 +72,10 @@ class OctetData } /** - * Set the buffer to the input buffer/length if the input has length in the interval [1,255] + * Set the octet data to the input buffer + * + * If the length is 0, the default value of [0x00] is assigned + * If the length is > 255, only the first 255 bytes are copied * * @param input the input data to copy into this object * @@ -80,7 +84,10 @@ class OctetData bool Set(const openpal::RSlice& input); /** - * Set the buffer equal to the supplied c-string if the input string has length in the interval [1,255] + * Set the buffer equal to the supplied c-string + * + * If the length is 0, the default value of [0x00] is assigned + * If the length is > 255, only the first 255 bytes are copied * * @param input c-style string to copy into this object * diff --git a/cpp/libs/include/opendnp3/app/OctetString.h b/cpp/libs/include/opendnp3/app/OctetString.h index 00474efa10..d2ce984176 100644 --- a/cpp/libs/include/opendnp3/app/OctetString.h +++ b/cpp/libs/include/opendnp3/app/OctetString.h @@ -34,7 +34,7 @@ class OctetString : public OctetData public: /** - * Construct with default length of 1 + * Construct with a default value of [0x00] (length == 1) */ OctetString() : OctetData() {} @@ -50,8 +50,8 @@ class OctetString : public OctetData * * strlen() is used internally to determine the length * - * If the length is 0, it is defaulted to 1 - * If the length is > 255, the first 255 bytes are copied. + * If the length is 0, the default value of [0x00] is assigned + * If the length is > 255, only the first 255 bytes are copied * * The null terminator is NOT copied as part of buffer */ @@ -61,8 +61,8 @@ class OctetString : public OctetData /** * Construct from read-only buffer slice * - * If the length is 0, it is defaulted to 1 - * If the length is > 255, the first 255 bytes are copied. + * If the length is 0, the default value of [0x00] is assigned + * If the length is > 255, only the first 255 bytes are copied */ OctetString(const openpal::RSlice& buffer) : OctetData(buffer) {} diff --git a/cpp/libs/src/opendnp3/app/OctetData.cpp b/cpp/libs/src/opendnp3/app/OctetData.cpp index da55585b78..2391a5ca71 100644 --- a/cpp/libs/src/opendnp3/app/OctetData.cpp +++ b/cpp/libs/src/opendnp3/app/OctetData.cpp @@ -53,22 +53,27 @@ OctetData::OctetData(const RSlice& input) : bool OctetData::Set(const openpal::RSlice& input) { - if ((input.Size() < 1) || (input.Size() > MAX_SIZE)) return false; - else + if (input.IsEmpty()) { - auto dest = buffer.GetWSlice(); - input.CopyTo(dest); - this->size = static_cast(input.Size()); - return true; + this->size = 0; + this->buffer.GetWSlice()[0] = 0x00; + return false; } + + const bool is_oversized = input.Size() > MAX_SIZE; + const uint8_t usable_size = is_oversized ? MAX_SIZE : static_cast(input.Size()); + + auto dest = this->buffer.GetWSlice(); + input.Take(usable_size).CopyTo(dest); + this->size = usable_size; + return !is_oversized; } bool OctetData::Set(const char* input) { const size_t length = strlen(input); - if (length > MAX_SIZE) return false; - return this->Set(openpal::RSlice(reinterpret_cast(input), static_cast(length))); + return this->Set(openpal::RSlice(reinterpret_cast(input), static_cast(length > MAX_SIZE ? MAX_SIZE : length))); } openpal::RSlice OctetData::ToRSlice() const