You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@plc4x.apache.org by 洪锦琳 <my...@gmail.com> on 2022/04/22 08:16:07 UTC

Reads of type bool have different results

Hello everyone!
First, thanks to Chris for his reply in my PR, so here I am!

Issue:
1 When I read 400006:BOOL[1], it will get the last bit of 400006(2bytes).
Here is the source code:
github.com/apache/plc4x/plc4go/internal/plc4go/modbus/readwrite/model/DataItem.go

case dataType == ModbusDataType_BOOL && numberOfValues == uint16(1): // BOOL
   // Reserved Field (Just skip the bytes)
   if _, _err := readBuffer.ReadUint16("reserved", 15); _err != nil {
      return nil, errors.Wrap(_err, "Error parsing reserved field")
   }

   // Simple Field (value)
   value, _valueErr := readBuffer.ReadBit("value")
   if _valueErr != nil {
      return nil, errors.Wrap(_valueErr, "Error parsing 'value' field")
   }
   readBuffer.CloseContext("DataItem")
   return values.NewPlcBOOL(value), nil


2 When I read 400006:BOOL[2], I get 400006 and 400007, the first two bits
of a total of 4 bytes.
Source code:
github.com/apache/plc4x/plc4go/internal/plc4go/modbus/readwrite/model/DataItem.go

case dataType == ModbusDataType_BOOL: // List
   // Array Field (value)
   var value []api.PlcValue
   for i := 0; i < int(numberOfValues); i++ {
      _item, _itemErr := readBuffer.ReadBit("value")
      if _itemErr != nil {
         return nil, errors.Wrap(_itemErr, "Error parsing 'value' field")
      }
      value = append(value, values.NewPlcBOOL(_item))
   }
   readBuffer.CloseContext("DataItem")
   return values.NewPlcList(value), nil

I thought it was 400006:BOOL[1] to get the 2nd bit of 400006, but the "[1]"
match in the regular is "quantity". So, I'm not sure how to fix this
problem correctly. Let's discuss it here.
github.com/apache/plc4x/plc4go/internal/plc4go/modbus/FieldHandler.go

generalFixedDigitAddressPattern :=
`(?P<address>\d{4,5})?(:(?P<datatype>[a-zA-Z_]+))?(\[(?P<quantity>\d+)])?$`

Re: Reads of type bool have different results

Posted by 洪锦琳 <my...@gmail.com>.
Okay, thank you

Christofer Dutz <ch...@c-ware.de> 於 2022年4月22日 週五 下午6:12寫道:

> Hi Myhongk,
>
> Well, you're never alone and we're always there to help you. So no
> worries, you're not going to break anything.
>
> Right now initially it was the plan to read the 10 least significant bit's
> of the register in that case, but from the code it's reading the most
> significant ones (Which I'm still not 100% sure which should be the right
> thing to do). What's definitely not right, is that we're not consuming the
> unnecessary bits at the end. So, when consuming 10 bits, we definitely need
> to consume the remaining 6 bits or the parser will be out of sync. But I
> guess as we're currently only reading one item at a time, this is probably
> not an issue, but it's still not formally correct.
>
> I think first we need to decide which path to go ...
>
> I'll start a DISCUSS thread on this so the others can jump in.
>
> Chris
>
>
>
>
> -----Original Message-----
> From: jl hong <my...@gmail.com>
> Sent: Freitag, 22. April 2022 11:37
> To: dev@plc4x.apache.org
> Subject: Re: Reads of type bool have different results
>
> Thanks for the reply, Chris.
> 1 Myhongk is ok, haha
> 2 It is my honor to be a regular contributor to open source projects.I'm
> just worried if I'm up to the task.
> 3 Thanks for the detailed answer on BOOL type, I will try to fix this bug.
> Let me confirm, 400006:BOOL[10], it means read the last 1-10 bits, right?
> For example, 10101010 10101010, the read is 10 10101010, in the program
> like list[0,1,0,1,0,1,0,1,0,1] Am I understanding correctly?
> Also, please tell me where I can learn the define of BOOL[2] is getting
> the last two bits of two registers
>
> Christofer Dutz <ch...@c-ware.de> 於 2022年4月22日 週五 下午4:30寫道:
>
> > Hi Myhong (Hope that's correct)
> >
> > First of all, welcome to our cool project ... I hope you'll like it
> > here and become a regular contributor ;-)
> >
> > Regarding BOOL[2] getting the last two bits of two registers is
> > definitely not the way it should be. It should be returning the last
> > two bits of one register and only span to the next if reading more
> > than 8. We should probably track this in an issue and fix it (if
> > you've already got an idea ... PRs are always welcome :-))
> >
> > However not 100% sure how we should generally do this. Perhaps [1]
> > should return the first bit and add lesser significant bits as the
> > array grows. I think I did it the way it's currently implemented, as
> > when using registers to store Boolean values I usually store 0 and 1 in
> them.
> >
> > Other protocols (s7) have the concept of a bit-address. Perhaps
> > artificially extending the modbus protocol with this concept would be
> > an option.
> >
> > Taking your example:
> >
> > 400006.1:BOOL[2]
> >
> > Could then read two bits starting at the second-most significant bit.
> >
> > 0XX00000 00000000
> >
> > Would that work for you? But in that case we probably should invert
> > how we read "BOOL" from interpreting:
> >
> > 00000000 000000001 as true to 10000000 00000000
> >
> > But not sure on how this would break other options.
> >
> > Chris
> >
> >
> > -----Original Message-----
> > From: 洪锦琳 <my...@gmail.com>
> > Sent: Freitag, 22. April 2022 10:16
> > To: dev@plc4x.apache.org
> > Subject: Reads of type bool have different results
> >
> > Hello everyone!
> > First, thanks to Chris for his reply in my PR, so here I am!
> >
> > Issue:
> > 1 When I read 400006:BOOL[1], it will get the last bit of 400006(2bytes).
> > Here is the source code:
> >
> > github.com/apache/plc4x/plc4go/internal/plc4go/modbus/readwrite/model/
> > DataItem.go
> >
> > case dataType == ModbusDataType_BOOL && numberOfValues == uint16(1):
> > // BOOL
> >    // Reserved Field (Just skip the bytes)
> >    if _, _err := readBuffer.ReadUint16("reserved", 15); _err != nil {
> >       return nil, errors.Wrap(_err, "Error parsing reserved field")
> >    }
> >
> >    // Simple Field (value)
> >    value, _valueErr := readBuffer.ReadBit("value")
> >    if _valueErr != nil {
> >       return nil, errors.Wrap(_valueErr, "Error parsing 'value' field")
> >    }
> >    readBuffer.CloseContext("DataItem")
> >    return values.NewPlcBOOL(value), nil
> >
> >
> > 2 When I read 400006:BOOL[2], I get 400006 and 400007, the first two
> > bits of a total of 4 bytes.
> > Source code:
> >
> > github.com/apache/plc4x/plc4go/internal/plc4go/modbus/readwrite/model/
> > DataItem.go
> >
> > case dataType == ModbusDataType_BOOL: // List
> >    // Array Field (value)
> >    var value []api.PlcValue
> >    for i := 0; i < int(numberOfValues); i++ {
> >       _item, _itemErr := readBuffer.ReadBit("value")
> >       if _itemErr != nil {
> >          return nil, errors.Wrap(_itemErr, "Error parsing 'value' field")
> >       }
> >       value = append(value, values.NewPlcBOOL(_item))
> >    }
> >    readBuffer.CloseContext("DataItem")
> >    return values.NewPlcList(value), nil
> >
> > I thought it was 400006:BOOL[1] to get the 2nd bit of 400006, but the
> "[1]"
> > match in the regular is "quantity". So, I'm not sure how to fix this
> > problem correctly. Let's discuss it here.
> > github.com/apache/plc4x/plc4go/internal/plc4go/modbus/FieldHandler.go
> >
> > generalFixedDigitAddressPattern :=
> > `(?P<address>\d{4,5})?(:(?P<datatype>[a-zA-Z_]+))?(\[(?P<quantity>\d+)
> > ])?$`
> >
>

RE: Reads of type bool have different results

Posted by Christofer Dutz <ch...@c-ware.de>.
Hi Myhongk,

Well, you're never alone and we're always there to help you. So no worries, you're not going to break anything.

Right now initially it was the plan to read the 10 least significant bit's of the register in that case, but from the code it's reading the most significant ones (Which I'm still not 100% sure which should be the right thing to do). What's definitely not right, is that we're not consuming the unnecessary bits at the end. So, when consuming 10 bits, we definitely need to consume the remaining 6 bits or the parser will be out of sync. But I guess as we're currently only reading one item at a time, this is probably not an issue, but it's still not formally correct.

I think first we need to decide which path to go ... 

I'll start a DISCUSS thread on this so the others can jump in.

Chris




-----Original Message-----
From: jl hong <my...@gmail.com> 
Sent: Freitag, 22. April 2022 11:37
To: dev@plc4x.apache.org
Subject: Re: Reads of type bool have different results

Thanks for the reply, Chris.
1 Myhongk is ok, haha
2 It is my honor to be a regular contributor to open source projects.I'm just worried if I'm up to the task.
3 Thanks for the detailed answer on BOOL type, I will try to fix this bug.
Let me confirm, 400006:BOOL[10], it means read the last 1-10 bits, right?
For example, 10101010 10101010, the read is 10 10101010, in the program  like list[0,1,0,1,0,1,0,1,0,1] Am I understanding correctly?
Also, please tell me where I can learn the define of BOOL[2] is getting the last two bits of two registers

Christofer Dutz <ch...@c-ware.de> 於 2022年4月22日 週五 下午4:30寫道:

> Hi Myhong (Hope that's correct)
>
> First of all, welcome to our cool project ... I hope you'll like it 
> here and become a regular contributor ;-)
>
> Regarding BOOL[2] getting the last two bits of two registers is 
> definitely not the way it should be. It should be returning the last 
> two bits of one register and only span to the next if reading more 
> than 8. We should probably track this in an issue and fix it (if 
> you've already got an idea ... PRs are always welcome :-))
>
> However not 100% sure how we should generally do this. Perhaps [1] 
> should return the first bit and add lesser significant bits as the 
> array grows. I think I did it the way it's currently implemented, as 
> when using registers to store Boolean values I usually store 0 and 1 in them.
>
> Other protocols (s7) have the concept of a bit-address. Perhaps 
> artificially extending the modbus protocol with this concept would be 
> an option.
>
> Taking your example:
>
> 400006.1:BOOL[2]
>
> Could then read two bits starting at the second-most significant bit.
>
> 0XX00000 00000000
>
> Would that work for you? But in that case we probably should invert 
> how we read "BOOL" from interpreting:
>
> 00000000 000000001 as true to 10000000 00000000
>
> But not sure on how this would break other options.
>
> Chris
>
>
> -----Original Message-----
> From: 洪锦琳 <my...@gmail.com>
> Sent: Freitag, 22. April 2022 10:16
> To: dev@plc4x.apache.org
> Subject: Reads of type bool have different results
>
> Hello everyone!
> First, thanks to Chris for his reply in my PR, so here I am!
>
> Issue:
> 1 When I read 400006:BOOL[1], it will get the last bit of 400006(2bytes).
> Here is the source code:
>
> github.com/apache/plc4x/plc4go/internal/plc4go/modbus/readwrite/model/
> DataItem.go
>
> case dataType == ModbusDataType_BOOL && numberOfValues == uint16(1): 
> // BOOL
>    // Reserved Field (Just skip the bytes)
>    if _, _err := readBuffer.ReadUint16("reserved", 15); _err != nil {
>       return nil, errors.Wrap(_err, "Error parsing reserved field")
>    }
>
>    // Simple Field (value)
>    value, _valueErr := readBuffer.ReadBit("value")
>    if _valueErr != nil {
>       return nil, errors.Wrap(_valueErr, "Error parsing 'value' field")
>    }
>    readBuffer.CloseContext("DataItem")
>    return values.NewPlcBOOL(value), nil
>
>
> 2 When I read 400006:BOOL[2], I get 400006 and 400007, the first two 
> bits of a total of 4 bytes.
> Source code:
>
> github.com/apache/plc4x/plc4go/internal/plc4go/modbus/readwrite/model/
> DataItem.go
>
> case dataType == ModbusDataType_BOOL: // List
>    // Array Field (value)
>    var value []api.PlcValue
>    for i := 0; i < int(numberOfValues); i++ {
>       _item, _itemErr := readBuffer.ReadBit("value")
>       if _itemErr != nil {
>          return nil, errors.Wrap(_itemErr, "Error parsing 'value' field")
>       }
>       value = append(value, values.NewPlcBOOL(_item))
>    }
>    readBuffer.CloseContext("DataItem")
>    return values.NewPlcList(value), nil
>
> I thought it was 400006:BOOL[1] to get the 2nd bit of 400006, but the "[1]"
> match in the regular is "quantity". So, I'm not sure how to fix this 
> problem correctly. Let's discuss it here.
> github.com/apache/plc4x/plc4go/internal/plc4go/modbus/FieldHandler.go
>
> generalFixedDigitAddressPattern :=
> `(?P<address>\d{4,5})?(:(?P<datatype>[a-zA-Z_]+))?(\[(?P<quantity>\d+)
> ])?$`
>

Re: Reads of type bool have different results

Posted by jl hong <my...@gmail.com>.
Thanks for the reply, Chris.
1 Myhongk is ok, haha
2 It is my honor to be a regular contributor to open source projects.I'm
just worried if I'm up to the task.
3 Thanks for the detailed answer on BOOL type, I will try to fix this bug.
Let me confirm, 400006:BOOL[10], it means read the last 1-10 bits, right?
For example, 10101010 10101010, the read is 10 10101010, in the program
 like list[0,1,0,1,0,1,0,1,0,1]
Am I understanding correctly?
Also, please tell me where I can learn the define of BOOL[2] is getting the
last two bits of two registers

Christofer Dutz <ch...@c-ware.de> 於 2022年4月22日 週五 下午4:30寫道:

> Hi Myhong (Hope that's correct)
>
> First of all, welcome to our cool project ... I hope you'll like it here
> and become a regular contributor ;-)
>
> Regarding BOOL[2] getting the last two bits of two registers is definitely
> not the way it should be. It should be returning the last two bits of one
> register and only span to the next if reading more than 8. We should
> probably track this in an issue and fix it (if you've already got an idea
> ... PRs are always welcome :-))
>
> However not 100% sure how we should generally do this. Perhaps [1] should
> return the first bit and add lesser significant bits as the array grows. I
> think I did it the way it's currently implemented, as when using registers
> to store Boolean values I usually store 0 and 1 in them.
>
> Other protocols (s7) have the concept of a bit-address. Perhaps
> artificially extending the modbus protocol with this concept would be an
> option.
>
> Taking your example:
>
> 400006.1:BOOL[2]
>
> Could then read two bits starting at the second-most significant bit.
>
> 0XX00000 00000000
>
> Would that work for you? But in that case we probably should invert how we
> read "BOOL" from interpreting:
>
> 00000000 000000001 as true to 10000000 00000000
>
> But not sure on how this would break other options.
>
> Chris
>
>
> -----Original Message-----
> From: 洪锦琳 <my...@gmail.com>
> Sent: Freitag, 22. April 2022 10:16
> To: dev@plc4x.apache.org
> Subject: Reads of type bool have different results
>
> Hello everyone!
> First, thanks to Chris for his reply in my PR, so here I am!
>
> Issue:
> 1 When I read 400006:BOOL[1], it will get the last bit of 400006(2bytes).
> Here is the source code:
>
> github.com/apache/plc4x/plc4go/internal/plc4go/modbus/readwrite/model/DataItem.go
>
> case dataType == ModbusDataType_BOOL && numberOfValues == uint16(1): //
> BOOL
>    // Reserved Field (Just skip the bytes)
>    if _, _err := readBuffer.ReadUint16("reserved", 15); _err != nil {
>       return nil, errors.Wrap(_err, "Error parsing reserved field")
>    }
>
>    // Simple Field (value)
>    value, _valueErr := readBuffer.ReadBit("value")
>    if _valueErr != nil {
>       return nil, errors.Wrap(_valueErr, "Error parsing 'value' field")
>    }
>    readBuffer.CloseContext("DataItem")
>    return values.NewPlcBOOL(value), nil
>
>
> 2 When I read 400006:BOOL[2], I get 400006 and 400007, the first two bits
> of a total of 4 bytes.
> Source code:
>
> github.com/apache/plc4x/plc4go/internal/plc4go/modbus/readwrite/model/DataItem.go
>
> case dataType == ModbusDataType_BOOL: // List
>    // Array Field (value)
>    var value []api.PlcValue
>    for i := 0; i < int(numberOfValues); i++ {
>       _item, _itemErr := readBuffer.ReadBit("value")
>       if _itemErr != nil {
>          return nil, errors.Wrap(_itemErr, "Error parsing 'value' field")
>       }
>       value = append(value, values.NewPlcBOOL(_item))
>    }
>    readBuffer.CloseContext("DataItem")
>    return values.NewPlcList(value), nil
>
> I thought it was 400006:BOOL[1] to get the 2nd bit of 400006, but the "[1]"
> match in the regular is "quantity". So, I'm not sure how to fix this
> problem correctly. Let's discuss it here.
> github.com/apache/plc4x/plc4go/internal/plc4go/modbus/FieldHandler.go
>
> generalFixedDigitAddressPattern :=
> `(?P<address>\d{4,5})?(:(?P<datatype>[a-zA-Z_]+))?(\[(?P<quantity>\d+)])?$`
>

RE: Reads of type bool have different results

Posted by Christofer Dutz <ch...@c-ware.de>.
Hi Myhong (Hope that's correct)

First of all, welcome to our cool project ... I hope you'll like it here and become a regular contributor ;-)

Regarding BOOL[2] getting the last two bits of two registers is definitely not the way it should be. It should be returning the last two bits of one register and only span to the next if reading more than 8. We should probably track this in an issue and fix it (if you've already got an idea ... PRs are always welcome :-))

However not 100% sure how we should generally do this. Perhaps [1] should return the first bit and add lesser significant bits as the array grows. I think I did it the way it's currently implemented, as when using registers to store Boolean values I usually store 0 and 1 in them. 

Other protocols (s7) have the concept of a bit-address. Perhaps artificially extending the modbus protocol with this concept would be an option.

Taking your example:

400006.1:BOOL[2]

Could then read two bits starting at the second-most significant bit.

0XX00000 00000000

Would that work for you? But in that case we probably should invert how we read "BOOL" from interpreting:

00000000 000000001 as true to 10000000 00000000

But not sure on how this would break other options.

Chris


-----Original Message-----
From: 洪锦琳 <my...@gmail.com> 
Sent: Freitag, 22. April 2022 10:16
To: dev@plc4x.apache.org
Subject: Reads of type bool have different results

Hello everyone!
First, thanks to Chris for his reply in my PR, so here I am!

Issue:
1 When I read 400006:BOOL[1], it will get the last bit of 400006(2bytes).
Here is the source code:
github.com/apache/plc4x/plc4go/internal/plc4go/modbus/readwrite/model/DataItem.go

case dataType == ModbusDataType_BOOL && numberOfValues == uint16(1): // BOOL
   // Reserved Field (Just skip the bytes)
   if _, _err := readBuffer.ReadUint16("reserved", 15); _err != nil {
      return nil, errors.Wrap(_err, "Error parsing reserved field")
   }

   // Simple Field (value)
   value, _valueErr := readBuffer.ReadBit("value")
   if _valueErr != nil {
      return nil, errors.Wrap(_valueErr, "Error parsing 'value' field")
   }
   readBuffer.CloseContext("DataItem")
   return values.NewPlcBOOL(value), nil


2 When I read 400006:BOOL[2], I get 400006 and 400007, the first two bits of a total of 4 bytes.
Source code:
github.com/apache/plc4x/plc4go/internal/plc4go/modbus/readwrite/model/DataItem.go

case dataType == ModbusDataType_BOOL: // List
   // Array Field (value)
   var value []api.PlcValue
   for i := 0; i < int(numberOfValues); i++ {
      _item, _itemErr := readBuffer.ReadBit("value")
      if _itemErr != nil {
         return nil, errors.Wrap(_itemErr, "Error parsing 'value' field")
      }
      value = append(value, values.NewPlcBOOL(_item))
   }
   readBuffer.CloseContext("DataItem")
   return values.NewPlcList(value), nil

I thought it was 400006:BOOL[1] to get the 2nd bit of 400006, but the "[1]"
match in the regular is "quantity". So, I'm not sure how to fix this problem correctly. Let's discuss it here.
github.com/apache/plc4x/plc4go/internal/plc4go/modbus/FieldHandler.go

generalFixedDigitAddressPattern :=
`(?P<address>\d{4,5})?(:(?P<datatype>[a-zA-Z_]+))?(\[(?P<quantity>\d+)])?$`

Re: Reads of type bool have different results

Posted by 洪锦琳 <my...@gmail.com>.
Sorry, for the title.
My first email was sent back because it was over the word count (because I
added a screenshot), and I was in a hurry to send a second email, so I
forgot to change the title, so sorry about that!
Original email title: [plc4go][issue]Reads of type bool have different
results

洪锦琳 <my...@gmail.com> 於 2022年4月22日 週五 下午4:16寫道:

> Hello everyone!
> First, thanks to Chris for his reply in my PR, so here I am!
>
> Issue:
> 1 When I read 400006:BOOL[1], it will get the last bit of 400006(2bytes).
> Here is the source code:
>
> github.com/apache/plc4x/plc4go/internal/plc4go/modbus/readwrite/model/DataItem.go
>
> case dataType == ModbusDataType_BOOL && numberOfValues == uint16(1): // BOOL
>    // Reserved Field (Just skip the bytes)
>    if _, _err := readBuffer.ReadUint16("reserved", 15); _err != nil {
>       return nil, errors.Wrap(_err, "Error parsing reserved field")
>    }
>
>    // Simple Field (value)
>    value, _valueErr := readBuffer.ReadBit("value")
>    if _valueErr != nil {
>       return nil, errors.Wrap(_valueErr, "Error parsing 'value' field")
>    }
>    readBuffer.CloseContext("DataItem")
>    return values.NewPlcBOOL(value), nil
>
>
> 2 When I read 400006:BOOL[2], I get 400006 and 400007, the first two bits
> of a total of 4 bytes.
> Source code:
>
> github.com/apache/plc4x/plc4go/internal/plc4go/modbus/readwrite/model/DataItem.go
>
> case dataType == ModbusDataType_BOOL: // List
>    // Array Field (value)
>    var value []api.PlcValue
>    for i := 0; i < int(numberOfValues); i++ {
>       _item, _itemErr := readBuffer.ReadBit("value")
>       if _itemErr != nil {
>          return nil, errors.Wrap(_itemErr, "Error parsing 'value' field")
>       }
>       value = append(value, values.NewPlcBOOL(_item))
>    }
>    readBuffer.CloseContext("DataItem")
>    return values.NewPlcList(value), nil
>
> I thought it was 400006:BOOL[1] to get the 2nd bit of 400006, but the "[1]"
> match in the regular is "quantity". So, I'm not sure how to fix this
> problem correctly. Let's discuss it here.
> github.com/apache/plc4x/plc4go/internal/plc4go/modbus/FieldHandler.go
>
> generalFixedDigitAddressPattern := `(?P<address>\d{4,5})?(:(?P<datatype>[a-zA-Z_]+))?(\[(?P<quantity>\d+)])?$`
>
>