oritbenhemo / nmodbus

Automatically exported from code.google.com/p/nmodbus
0 stars 0 forks source link

Repost of Issue 22: ModbusDataCollection.cs ArgumentOutOfRangeException #24

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
(this is in reply to scott's message below) (sorry for not following the
format)

yes, i understand, but when you set the coil using the address of 1, it
sets the coil with address 2 (index 1 in the array)- so essentially, the
item with index 0 (which would be coil 1) in the array is stranded- you can
never set it.  the best way to reproduce this is to list all the registers
by address.  then try to set address 1, you will see that the value of
address 2 has changed. 

so you have to adjust for the base 1 numbering or something.  hopefully i
am not mistaken, i am pretty sure i am not since i am using the library to
build a full-featured slave and the coil (address 1) with index 0 in the
array is unreachable due to the check in the code.  i have 3000 coils
numbered 0 through 2999 in the array with the addresses of 1 through 3000.
 Once again if i try a coil using address 1, it sets the coil with the
address of 2...

 Issue 22: ModbusDataCollection.cs ArgumentOutOfRangeException
http://code.google.com/p/nmodbus/issues/detail?id=22

Comment #1 by sjalex:
Actually this is by design.

1) In a MODBUS PDU data is addressed from 0 to 65535 (0 origin)
2) In the MODBUS data Model each element within a data block is
numbered from 1 to n
(1 origin)
3) .NET collections are of 0 origin

In order to be true to the Modbus data model I created the
ModbusDataCollection, a 1
origin collection.

http://modbus.org/docs/Modbus_Application_Protocol_V1_1b.pdf for more details.

Scott

Original issue reported on code.google.com by robma...@gmail.com on 13 Dec 2007 at 3:17

GoogleCodeExporter commented 9 years ago
You are correct and it is by design that the 0 index value in the DataStore is
stranded, it will never be read from or written to using the Modbus functions.

I think this unit test may speak more clearly

/// <summary>
/// http://modbus.org/docs/Modbus_Application_Protocol_V1_1b.pdf
/// In the PDU Coils are addressed starting at zero. Therefore coils numbered 
1-16
are addressed as 0-15.
/// So reading Modbus address 0 should get you array index 1 in the DataStore.
/// This implies that the DataStore array index 0 can never be used.
/// </summary>
[Test]
public void TestReadMapping()
{
    DataStore dataStore = DataStoreFactory.CreateDefaultDataStore();
    dataStore.HoldingRegisters.Insert(1, 45);
    dataStore.HoldingRegisters.Insert(2, 42);

    Assert.AreEqual(45, DataStore.ReadData<RegisterCollection, ushort>(dataStore,
dataStore.HoldingRegisters, 0, 1)[0]);
    Assert.AreEqual(42, DataStore.ReadData<RegisterCollection, ushort>(dataStore,
dataStore.HoldingRegisters, 1, 1)[0]);
}

Original comment by sja...@gmail.com on 26 Mar 2008 at 12:59

GoogleCodeExporter commented 9 years ago
Uh, you're charging down a totally unrelated path. Forget about 0 index.

"the best way to reproduce this is to list all the registers
by address.  then try to set address 1, you will see that the value of
address 2 has changed."  

I've already fixed the bug and have been using it- if you want to leave the bug 
in
there then that's fine.  Sooner or later someone else will find it and you can 
deal
with them.

Original comment by robma...@gmail.com on 26 Mar 2008 at 1:26

GoogleCodeExporter commented 9 years ago
ok, i'm heeding your advice so correct me if I'm misunderstanding you.

// Create a DataStore and set some initial values
var dataStore = DataStoreFactory.CreateTestDataStore();

// print some DataStore values
for (int i = 0; i < 5; i++)
       Debug.WriteLine(String.Format("Register value {0} = {1}", i,
dataStore.HoldingRegisters[i]));

The results are
Register value 0 = 0
Register value 1 = 1
Register value 2 = 2
Register value 3 = 3
Register value 4 = 4

// Now we use the WriteData method to set Modbus address 0 and reprint
the values
ushort smallestPossibleModbusAddress = 0;
DataStore.WriteData(dataStore, new RegisterCollection(42),
dataStore.HoldingRegisters, smallestPossibleModbusAddress);

for (int i = 0; i < 5; i++)
       Debug.WriteLine(String.Format("Register value {0} = {1}", i,
dataStore.HoldingRegisters[i]));

Register value 0 = 0
Register value 1 = 42
Register value 2 = 2
Register value 3 = 3
Register value 4 = 4

I'm saying these are the expected results. Are you disagreeing with
this output?

Scott

Original comment by sja...@gmail.com on 26 Mar 2008 at 1:55

GoogleCodeExporter commented 9 years ago
dude, are you reading my note?  "the best way to reproduce this is to list all 
the
registers
by address.  then try to set address 1, you will see that the value of
address 2 has changed." 

set address 1 NOT address 0.  forget about address 0.

Original comment by robma...@gmail.com on 26 Mar 2008 at 2:24

GoogleCodeExporter commented 9 years ago
Those are the expected results...I can't reproduce the bug I originally 
submitted- I
posted it so long ago that I can't remember where it was in my code.  I was 
actually
suprised to see your email yesterday.  
When i run a small piece of code in a console app against the coils everything 
works
fine.  So, sorry for wasting you time and taking a rough tone.  I only wanted 
to help
the project- as I think, overall, it is a great piece of code!  When I get the 
time,
I will see if I can't find the original problem and will post a more thorough 
piece
of code as an example (if indeed it's really a bug).

Thanks for your work. 

Original comment by robma...@gmail.com on 26 Mar 2008 at 2:49