Re: Inquiry about the mspec

2020-07-17 Thread Christofer Dutz
Hi Cesar,

a typeSwitch inside a typeSwitch never worked ... you need to introduce an 
intermediate type for that.

So instead of:

[discriminiatedType Hurz
..
  [typeSwitch 'something'
[case '1' HurzOne
  ...
  [typeSwitch 'else'
[case '2' HurzOneTwo
  ..
]
 ]
  ]
]

You would have to do:

[discriminiatedType Hurz
..
  [typeSwitch 'something'
[case '1' HurzOne
  [simple HurzOneDetail 'details']
]
  ]
]

[discriminiatedType HurzOneDetail
  ...
  [typeSwitch 'else'
[case '2' HurzOneTwo
  ..
]
]

Hope that was your question.


Chris



Am 17.07.20, 17:58 schrieb "Cesar Garcia" :

Hello Chris.

It's me again, :-)

You have an example that shows that the "typeSwitch" tab allows an internal
"typeSwitch" tab as presented.

I have tried many combinations and it does not work for me.

Thanking you for your help,

El lun., 15 jun. 2020 a las 3:21, Christofer Dutz (<
christofer.d...@c-ware.de>) escribió:

> Hi Cesar,
>
> I just noticed your discriminated types in [2] don't have names ... as for
> every case in the typeSwitch a new type is generated, you need to give 
them
> names.
> Please try that and check if the error goes away.
>
> Chris
>
>
> Am 14.06.20, 17:59 schrieb "Cesar Garcia" :
>
> Very grateful for your prompt response,
>
> Corrected at point [1]
>
> Unfortunately, the build process still fails when I add the
> 'typeSwitch' in
> the structure as shown in [2],
>
> I attach the message of error in [3].
>
> Thank you very much for any help you can provide,
>
> Best regards,
>
>
> [1]
> [discriminatedType 'S7Payload' [uint 8 'messageType', S7Parameter
> 'parameter']
> [typeSwitch 'parameter.discriminatorValues[0]', 'messageType'
> ['0x04','0x03' S7PayloadReadVarResponse
> [array S7VarPayloadDataItem 'items' count 'CAST(parameter,
> S7ParameterReadVarResponse).numItems' ['lastItem']]
> ]
> ['0x05','0x01' S7PayloadWriteVarRequest
> [array S7VarPayloadDataItem 'items' count
> 'COUNT(CAST(parameter, S7ParameterWriteVarRequest).items)'
> ['lastItem']]
> ]
> ['0x05','0x03' S7PayloadWriteVarResponse
> [array S7VarPayloadStatusItem 'items' count
> 'CAST(parameter,
> S7ParameterWriteVarResponse).numItems']
> ]
> ['0x00','0x07' S7PayloadUserData
> *[array S7PayloadUserDataItem 'items' count
> 'COUNT(CAST(parameter, S7ParameterUserData).items)'
> ['CAST(CAST(parameter,
> S7ParameterUserData).items[0],
>
> 
S7ParameterUserDataItemCPUFunctions).cpuFunctionType','CAST(CAST(parameter,
> S7ParameterUserData).items[0],
> S7ParameterUserDataItemCPUFunctions).cpuSubfunction'*]]
> ]
> ]
> ]
>
> [2]
> [discriminatedType 'S7PayloadUserDataItem' [uint 4 'cpuFunctionType',
> uint
> 8 'cpuSubfunction']
> [enum DataTransportErrorCode 'returnCode']
> [enum DataTransportSize  'transportSize']
> [implicit uint 16'dataLength' 'lengthInBytes - 4']
> [typeSwitch 'cpuSubfunction'
> ['0x01'
> [simple   SzlId  'szlId']
> [simple   uint 16'szlIndex']
> [typeSwitch 'cpuFunctionType'
> ['0x04' S7PayloadUserDataItemCpuFunctionReadSzlRequest
> ]
> ['0x08' 
S7PayloadUserDataItemCpuFunctionReadSzlResponse
> [constuint 16 'szlItemLength' '28']
> [implicit uint 16 'szlItemCount'  'COUNT(items)']
> [array SzlDataTreeItem 'items' count
> 'szlItemCount']
> ]
> ]
> ]
> ['0x02'
> *[simple   uint 16'Mode']*
> ]
> ]
> ]
>
>
> [3]
>
> --- plc4x-maven-plugin:1.2.0:generate-driver (generate-driver) @
> plc4j-driver-s7 ---
> Generating type DataItem
> Generating type COTPParameterDisconnectAdditionalInformation
> Generating type S7PayloadWriteVarRequest
>
> 
> BUILD FAILURE
>
> 
> Total time:  2.220 s
> Finished at: 2020-06-14T11:27:37-04:00
>
> 

Re: Inquiry about the mspec

2020-07-17 Thread Cesar Garcia
Hello Chris.

It's me again, :-)

You have an example that shows that the "typeSwitch" tab allows an internal
"typeSwitch" tab as presented.

I have tried many combinations and it does not work for me.

Thanking you for your help,

El lun., 15 jun. 2020 a las 3:21, Christofer Dutz (<
christofer.d...@c-ware.de>) escribió:

> Hi Cesar,
>
> I just noticed your discriminated types in [2] don't have names ... as for
> every case in the typeSwitch a new type is generated, you need to give them
> names.
> Please try that and check if the error goes away.
>
> Chris
>
>
> Am 14.06.20, 17:59 schrieb "Cesar Garcia" :
>
> Very grateful for your prompt response,
>
> Corrected at point [1]
>
> Unfortunately, the build process still fails when I add the
> 'typeSwitch' in
> the structure as shown in [2],
>
> I attach the message of error in [3].
>
> Thank you very much for any help you can provide,
>
> Best regards,
>
>
> [1]
> [discriminatedType 'S7Payload' [uint 8 'messageType', S7Parameter
> 'parameter']
> [typeSwitch 'parameter.discriminatorValues[0]', 'messageType'
> ['0x04','0x03' S7PayloadReadVarResponse
> [array S7VarPayloadDataItem 'items' count 'CAST(parameter,
> S7ParameterReadVarResponse).numItems' ['lastItem']]
> ]
> ['0x05','0x01' S7PayloadWriteVarRequest
> [array S7VarPayloadDataItem 'items' count
> 'COUNT(CAST(parameter, S7ParameterWriteVarRequest).items)'
> ['lastItem']]
> ]
> ['0x05','0x03' S7PayloadWriteVarResponse
> [array S7VarPayloadStatusItem 'items' count
> 'CAST(parameter,
> S7ParameterWriteVarResponse).numItems']
> ]
> ['0x00','0x07' S7PayloadUserData
> *[array S7PayloadUserDataItem 'items' count
> 'COUNT(CAST(parameter, S7ParameterUserData).items)'
> ['CAST(CAST(parameter,
> S7ParameterUserData).items[0],
>
> S7ParameterUserDataItemCPUFunctions).cpuFunctionType','CAST(CAST(parameter,
> S7ParameterUserData).items[0],
> S7ParameterUserDataItemCPUFunctions).cpuSubfunction'*]]
> ]
> ]
> ]
>
> [2]
> [discriminatedType 'S7PayloadUserDataItem' [uint 4 'cpuFunctionType',
> uint
> 8 'cpuSubfunction']
> [enum DataTransportErrorCode 'returnCode']
> [enum DataTransportSize  'transportSize']
> [implicit uint 16'dataLength' 'lengthInBytes - 4']
> [typeSwitch 'cpuSubfunction'
> ['0x01'
> [simple   SzlId  'szlId']
> [simple   uint 16'szlIndex']
> [typeSwitch 'cpuFunctionType'
> ['0x04' S7PayloadUserDataItemCpuFunctionReadSzlRequest
> ]
> ['0x08' S7PayloadUserDataItemCpuFunctionReadSzlResponse
> [constuint 16 'szlItemLength' '28']
> [implicit uint 16 'szlItemCount'  'COUNT(items)']
> [array SzlDataTreeItem 'items' count
> 'szlItemCount']
> ]
> ]
> ]
> ['0x02'
> *[simple   uint 16'Mode']*
> ]
> ]
> ]
>
>
> [3]
>
> --- plc4x-maven-plugin:1.2.0:generate-driver (generate-driver) @
> plc4j-driver-s7 ---
> Generating type DataItem
> Generating type COTPParameterDisconnectAdditionalInformation
> Generating type S7PayloadWriteVarRequest
>
> 
> BUILD FAILURE
>
> 
> Total time:  2.220 s
> Finished at: 2020-06-14T11:27:37-04:00
>
> 
> Failed to execute goal
> org.apache.plc4x.plugins:plc4x-maven-plugin:1.2.0:generate-driver
> (generate-driver) on project plc4j-driver-s7: Error generating sources:
> Error generating output for type 'S7PayloadWriteVarRequest': Java
> method
>
> "org.apache.plc4x.language.java.JavaLanguageTemplateHelper.getArgumentType(org.apache.plc4x.plugins.codegenerator.types.references.TypeReference,
> int)" threw an exception when invoked on
> org.apache.plc4x.language.java.JavaLanguageTemplateHelper object
> "org.apache.plc4x.language.java.JavaLanguageTemplateHelper@5fa0141f";
> see
> cause exception in the Java stack trace.
>
> 
> FTL stack trace ("~" means nesting-related):
> - Failed at: ${helper.getArgumentType(field.type, ...  [in template
> "templates/java/io-template.ftlh" at line 136, column 248]
> : Could not find definition of complex type S7VarPayloadDataItem
> -> [Help 1]
>
> To see the full stack trace of the errors, re-run Maven with the -e
> switch.
> Re-run Maven using the -X switch to enable full debug logging.
>
> 

Re: [DISCUSS] How about naming "first time contributors" in RELEASE_NOTES?

2020-07-17 Thread Julian Feinauer
I like it.

We have quite a big community of people who contributed "small" things but 
still they deserve it : )

Julian

Am 17.07.20, 14:43 schrieb "Christofer Dutz" :

Hi all,

I still remember how happy it made me to have my first contribution 
recognized in an open-source project. Even if it was a tiny one.

So I was thinking … how about we mention first time contributors in the 
RELEASE_NOTES? I wouldn’t mention what they did, how much it was, just list the 
names.

Of course, after asking them ;-)

Chris



[BUILD-STABLE]: Job 'PLC4X/PLC4X/develop [develop] [969]'

2020-07-17 Thread Apache Jenkins Server
BUILD-STABLE: Job 'PLC4X/PLC4X/develop [develop] [969]':

Is back to normal.

[DISCUSS] How about naming "first time contributors" in RELEASE_NOTES?

2020-07-17 Thread Christofer Dutz
Hi all,

I still remember how happy it made me to have my first contribution recognized 
in an open-source project. Even if it was a tiny one.

So I was thinking … how about we mention first time contributors in the 
RELEASE_NOTES? I wouldn’t mention what they did, how much it was, just list the 
names.

Of course, after asking them ;-)

Chris


Re: [jira] [Created] (PLC4X-214) [Modbus] Holding register addresses have an offset of 1 (Not reading the correct address)

2020-07-17 Thread Christofer Dutz
Hi Ben ... 

I just merged your pull request ... currently doing the full build with all 
tests but so-far it's looking good.
Thanks a lot for this.

Chris


Am 17.07.20, 10:34 schrieb "Christofer Dutz" :

Hi Ben, 

I just reviewed your changes and I like them a lot ... I did find some 
minor things that might deserve tweaking.
But thanks for that :)

Chris

Am 17.07.20, 05:14 schrieb "Ben Hutcheson" :

Hi,

Sure I'll give it a shot,

I have created a pull request to be able to use the (01, 0x1,
11, etc..)  address formats as well as to change the minimum 
address to
1. Corresponding to 01 or the first coil.
Please don't hold back on criticizing it, it is the only way I'll learn.

I'll take a look at adding the extended memory area next, I'm expecting 
it
will take me around a week.

Ben


On Thu, Jul 16, 2020 at 12:25 PM Christofer Dutz 

wrote:

> Aaaahh ... perfect
>
> Thanks Ben and Niclas.
> Guess I'll be doing some Modbus coding pretty soon :-)
>
> But if someone wants to try I'd be more than happy :)
>
> Chris
>
>
>
> Am 16.07.20, 11:48 schrieb "Ben Hutcheson" :
>
> Hi,
>
> *I think we have 6 separate memory areas. Do you have a mapping, 
That I
> could use? I mean which first digit represents which memory area?*
> I thought there were only 5 areas?
> 0x - Coils
> 1x - Inputs
> 3x - Input Registers
> 4x - Holding Registers
> 6x - Extended Registers
>
> I've seen the IEEE format for 32-bit floats, also another format 
that
> gets
> used is multiplying the float by 100 or 1000 and then dividing it 
by
> the
> same on the other end. e.g. 56.67 becomes 5667,
>
> Kind Regards
>
> Ben
>
> On Thu, Jul 16, 2020 at 4:10 AM Niclas Hedhman 

> wrote:
>
> > For floats, I have only seen IEEE format. But can't rule out 
other.
> >
> > On Thu, Jul 16, 2020, 14:57 Christofer Dutz <
> christofer.d...@c-ware.de>
> > wrote:
> >
> > > Guess it should be possible for plc4x to interpret INT as two
> shorts long
> > > as four... In that case it could probably also handle half
> precision
> > floats
> > > (16 bit), full floats and double, if the encoding is somewhat
> standard
> > > (which I assume it's not)
> > >
> > > Chris
> > > 
> > > Von: Niclas Hedhman 
> > > Gesendet: Donnerstag, 16. Juli 2020 08:33
> > > An: dev@plc4x.apache.org 
> > > Betreff: Re: [jira] [Created] (PLC4X-214) [Modbus] Holding 
register
> > > addresses have an offset of 1 (Not reading the correct 
address)
> > >
> > > To make things worse, there is equipment on the market with 
both
> 32-bit
> > > numbers as well IEEE floats.
> > >
> > > And many clients are incapable of doing something meaningful 
with
> > those...
> > >
> > >
> > > And then there is equipment that uses one register to 
indicate the
> > > magnitude of one or more other registers, say 1="divide by 1",
> 2="divide
> > by
> > > 10"...
> > >
> > >
> > >
> > > Niclas
> > >
> > > On Thu, Jul 16, 2020, 14:28 Christofer Dutz <
> christofer.d...@c-ware.de>
> > > wrote:
> > >
> > > > Hi Ben and Otto,
> > > >
> > > > First off all, thank you Ben for that very detailed 
explanation.
> It
> > does
> > > > seem as if we should extend the parser to support the 
different
> numeric
> > > > variants. I don't see any problems in supporting both the
> hex-like one
> > as
> > > > well as the pure numeric one.
> > > >
> > > > I think we have 6 separate memory areas. Do you have a 
mapping,
> That I
> > > > could use? I mean which first digit represents which memory 
area?
> > > >
> > > > @otto Modbus doesn't allow floats. Just bits (coils) and 
shorts
> > > > (registers)... Haven't seen a somewhat standard way to 
encode
> anything
> > > else.
> > > >
> > > > Chris
> > > > 
> > > > Von: Otto Fowler 
  

[BUILD-FAILURE]: Job 'PLC4X/PLC4X/develop [develop] [968]'

2020-07-17 Thread Apache Jenkins Server
BUILD-FAILURE: Job 'PLC4X/PLC4X/develop [develop] [968]':

Check console output at "https://builds.apache.org/job/PLC4X/job/PLC4X/job/develop/968/;>PLC4X/PLC4X/develop
 [develop] [968]"

[BUILD-FAILURE]: Job 'PLC4X/PLC4X/develop [develop] [967]'

2020-07-17 Thread Apache Jenkins Server
BUILD-FAILURE: Job 'PLC4X/PLC4X/develop [develop] [967]':

Check console output at "https://builds.apache.org/job/PLC4X/job/PLC4X/job/develop/967/;>PLC4X/PLC4X/develop
 [develop] [967]"

[GitHub] [plc4x] chrisdutz merged pull request #172: Feature/modbus add additional address formats and change lowest register to 1.

2020-07-17 Thread GitBox


chrisdutz merged pull request #172:
URL: https://github.com/apache/plc4x/pull/172


   



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [plc4x] chrisdutz merged pull request #171: [PLC4X-216]update IoTDB JDBC example and session API example; add the related doc on website

2020-07-17 Thread GitBox


chrisdutz merged pull request #171:
URL: https://github.com/apache/plc4x/pull/171


   



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [plc4x] jixuan1989 commented on pull request #171: [PLC4X-216]update IoTDB JDBC example and session API example; add the related doc on website

2020-07-17 Thread GitBox


jixuan1989 commented on pull request #171:
URL: https://github.com/apache/plc4x/pull/171#issuecomment-660012504


   > Probably might be a good idea to add the "useJDBC" to the options and 
allow switching from the command-line?
   
   good idea. done. 



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[jira] [Created] (PLC4X-217) ADS connection issue, Help wanted

2020-07-17 Thread Vikram Gopu (Jira)
Vikram Gopu created PLC4X-217:
-

 Summary: ADS connection issue, Help wanted
 Key: PLC4X-217
 URL: https://issues.apache.org/jira/browse/PLC4X-217
 Project: Apache PLC4X
  Issue Type: Bug
  Components: Driver-ADS
Affects Versions: 0.6.0
 Environment: Windows '10 PC
Reporter: Vikram Gopu


I ran twincat simulator on my local host machine with ip 192.168.x.x subnetwork 
and the similator has the ip address 172.21.97.81, and then i have used the ads 
server connection string: ads:tcp://localhost/172.21.97.81.1.1:851, which seems 
not to be connected and i receive the error message as shown in the logs. Can 
someone point out what the problem is or the bug is ?

 

Best Regards

Vikram Gopu 

 

 

 

[main] INFO org.apache.plc4x.java.PlcDriverManager - Instantiating new PLC 
Driver Manager with class loader 
jdk.internal.loader.ClassLoaders$AppClassLoader@2626b418[main] INFO 
org.apache.plc4x.java.PlcDriverManager - Instantiating new PLC Driver Manager 
with class loader 
jdk.internal.loader.ClassLoaders$AppClassLoader@2626b418[main] INFO 
org.apache.plc4x.java.PlcDriverManager - Registering available drivers...[main] 
INFO org.apache.plc4x.java.PlcDriverManager - Registering driver for Protocol 
modbus (Modbus (TCP / Serial))[main] INFO 
org.apache.plc4x.java.PlcDriverManager - Registering driver for Protocol s7 
(Siemens S7 (Basic))[main] INFO org.apache.plc4x.java.PlcDriverManager - 
Registering driver for Protocol ads (Beckhoff Twincat ADS)[main] INFO 
org.apache.plc4x.java.scraper.config.triggeredscraper.ScraperConfigurationTriggeredImpl
 - Assuming job as triggered job because triggerConfig has been set[main] INFO 
org.apache.plc4x.java.scraper.triggeredscraper.TriggeredScraperImpl - Starting 
jobs...[main] INFO 
org.apache.plc4x.java.scraper.triggeredscraper.TriggeredScraperImpl - Task 
TriggeredScraperTask\{driverManager=org.apache.plc4x.java.utils.connectionpool.PooledPlcDriverManager@4b9e255,
 jobName='ScheduleJob', connectionAlias='DeviceSource', 
connectionString='ads:tcp://localhost/172.21.97.81.1.1:851', 
requestTimeoutMs=1000, 
executorService=java.util.concurrent.ThreadPoolExecutor@5e57643e[Running, pool 
size = 0, active threads = 0, queued tasks = 0, completed tasks = 0], 
resultHandler=eu.cloudplug.cpe.plc4x.PLC4XScrapper$$Lambda$67/0x000800bcac40@133e16fd,
 
triggerHandler=org.apache.plc4x.java.scraper.triggeredscraper.triggerhandler.TriggerHandlerImpl@51b279c9}
 added to scheduling[triggeredscraper-scheduling-thread-1] WARN 
org.apache.plc4x.java.scraper.triggeredscraper.TriggeredScraperTask - Exception 
during scraping of Job ScheduleJob, Connection-Alias DeviceSource: 
Error-message: null - for stack-trace change logging to 
DEBUG[nioEventLoopGroup-3-1] WARN io.netty.channel.DefaultChannelPipeline - An 
exceptionCaught() event was fired, and it reached at the tail of the pipeline. 
It usually means the last handler in the pipeline did not handle the 
exception.io.netty.handler.codec.DecoderException: 
java.lang.IndexOutOfBoundsException at 
io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:98)
 at 
io.netty.handler.codec.MessageToMessageCodec.channelRead(MessageToMessageCodec.java:111)
 at 
io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:374)
 at 
io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:360)
 at 
io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:352)
 at 
io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:102)
 at 
io.netty.handler.codec.MessageToMessageCodec.channelRead(MessageToMessageCodec.java:111)
 at 
io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:374)
 at 
io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:360)
 at 
io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:352)
 at 
io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1421)
 at 
io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:374)
 at 
io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:360)
 at 
io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:930)
 at 
io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:163)
 at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:697) 
at 
io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:632)
 at 
io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:549) at 

[GitHub] [plc4x] hutcheb commented on a change in pull request #172: Feature/modbus add additional address formats and change lowest register to 1.

2020-07-17 Thread GitBox


hutcheb commented on a change in pull request #172:
URL: https://github.com/apache/plc4x/pull/172#discussion_r456333241



##
File path: 
plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/field/ModbusFieldDiscreteInput.java
##
@@ -26,21 +26,40 @@ Licensed to the Apache Software Foundation (ASF) under one
 public class ModbusFieldDiscreteInput extends ModbusField {
 
 public static final Pattern ADDRESS_PATTERN = 
Pattern.compile("discrete-input:" + ModbusField.ADDRESS_PATTERN);
+public static final Pattern ADDRESS_SHORTER_PATTERN = Pattern.compile("1" 
+ ModbusField.ADDRESS_PATTERN);
+public static final Pattern ADDRESS_SHORT_PATTERN = Pattern.compile("1x" + 
ModbusField.ADDRESS_PATTERN);
 
 public ModbusFieldDiscreteInput(int address, Integer quantity) {
 super(address, quantity);
 }
 
-public static ModbusFieldDiscreteInput of(String addressString) throws 
PlcInvalidFieldException {
-Matcher matcher = ADDRESS_PATTERN.matcher(addressString);
-if (!matcher.matches()) {
-throw new PlcInvalidFieldException(addressString, ADDRESS_PATTERN);
+public static boolean matches(String addressString) {
+return ADDRESS_PATTERN.matcher(addressString).matches() ||
+ADDRESS_SHORTER_PATTERN.matcher(addressString).matches() ||
+ADDRESS_SHORT_PATTERN.matcher(addressString).matches();
+}
+
+public static Matcher getMatcher(String addressString) throws 
PlcInvalidFieldException {
+Matcher matcher;
+if (ADDRESS_PATTERN.matcher(addressString).matches()) {
+  matcher = ADDRESS_PATTERN.matcher(addressString);
+} else if (ADDRESS_SHORT_PATTERN.matcher(addressString).matches()) {
+  matcher = ADDRESS_SHORT_PATTERN.matcher(addressString);
+} else if (ADDRESS_SHORTER_PATTERN.matcher(addressString).matches()) {
+  matcher = ADDRESS_SHORTER_PATTERN.matcher(addressString);
+} else {
+  throw new PlcInvalidFieldException(addressString, ADDRESS_PATTERN);
 }
-int address = Integer.parseInt(matcher.group("address"));
+return matcher;
+}
+
+public static ModbusFieldDiscreteInput of(String addressString) throws 
PlcInvalidFieldException {
+Matcher matcher = getMatcher(addressString);
+matcher.find();

Review comment:
   Yeah that makes more sense, I'll change that.





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [plc4x] chrisdutz commented on a change in pull request #172: Feature/modbus add additional address formats and change lowest register to 1.

2020-07-17 Thread GitBox


chrisdutz commented on a change in pull request #172:
URL: https://github.com/apache/plc4x/pull/172#discussion_r456329012



##
File path: 
plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/field/ModbusFieldDiscreteInput.java
##
@@ -26,21 +26,40 @@ Licensed to the Apache Software Foundation (ASF) under one
 public class ModbusFieldDiscreteInput extends ModbusField {
 
 public static final Pattern ADDRESS_PATTERN = 
Pattern.compile("discrete-input:" + ModbusField.ADDRESS_PATTERN);
+public static final Pattern ADDRESS_SHORTER_PATTERN = Pattern.compile("1" 
+ ModbusField.ADDRESS_PATTERN);
+public static final Pattern ADDRESS_SHORT_PATTERN = Pattern.compile("1x" + 
ModbusField.ADDRESS_PATTERN);
 
 public ModbusFieldDiscreteInput(int address, Integer quantity) {
 super(address, quantity);
 }
 
-public static ModbusFieldDiscreteInput of(String addressString) throws 
PlcInvalidFieldException {
-Matcher matcher = ADDRESS_PATTERN.matcher(addressString);
-if (!matcher.matches()) {
-throw new PlcInvalidFieldException(addressString, ADDRESS_PATTERN);
+public static boolean matches(String addressString) {
+return ADDRESS_PATTERN.matcher(addressString).matches() ||
+ADDRESS_SHORTER_PATTERN.matcher(addressString).matches() ||
+ADDRESS_SHORT_PATTERN.matcher(addressString).matches();
+}
+
+public static Matcher getMatcher(String addressString) throws 
PlcInvalidFieldException {
+Matcher matcher;
+if (ADDRESS_PATTERN.matcher(addressString).matches()) {
+  matcher = ADDRESS_PATTERN.matcher(addressString);
+} else if (ADDRESS_SHORT_PATTERN.matcher(addressString).matches()) {
+  matcher = ADDRESS_SHORT_PATTERN.matcher(addressString);
+} else if (ADDRESS_SHORTER_PATTERN.matcher(addressString).matches()) {
+  matcher = ADDRESS_SHORTER_PATTERN.matcher(addressString);
+} else {
+  throw new PlcInvalidFieldException(addressString, ADDRESS_PATTERN);
 }
-int address = Integer.parseInt(matcher.group("address"));
+return matcher;
+}
+
+public static ModbusFieldDiscreteInput of(String addressString) throws 
PlcInvalidFieldException {
+Matcher matcher = getMatcher(addressString);
+matcher.find();

Review comment:
   Ups seems that response wen't to the wrong comment ... sorry ... well I 
just noticed that IntelliJ was complaining ... And yeah ... if it wouldn't 
match, it wouldn't pass the getMatcher() call ... so how about instead of 
creating one matcher to check if it matches and then to return a new one ... 
how about returning the instance you used to check?

##
File path: 
plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/field/ModbusFieldCoil.java
##
@@ -26,17 +26,37 @@ Licensed to the Apache Software Foundation (ASF) under one
 public class ModbusFieldCoil extends ModbusField {
 
 public static final Pattern ADDRESS_PATTERN = Pattern.compile("coil:" + 
ModbusField.ADDRESS_PATTERN);
+public static final Pattern ADDRESS_SHORTER_PATTERN = Pattern.compile("0" 
+ ModbusField.ADDRESS_PATTERN);

Review comment:
   Yeah .. in that case I would prefer "5 or more" digits (You can probably 
just do 5-6 digits)





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [plc4x] hutcheb commented on a change in pull request #172: Feature/modbus add additional address formats and change lowest register to 1.

2020-07-17 Thread GitBox


hutcheb commented on a change in pull request #172:
URL: https://github.com/apache/plc4x/pull/172#discussion_r456321048



##
File path: 
plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/field/ModbusFieldDiscreteInput.java
##
@@ -26,21 +26,40 @@ Licensed to the Apache Software Foundation (ASF) under one
 public class ModbusFieldDiscreteInput extends ModbusField {
 
 public static final Pattern ADDRESS_PATTERN = 
Pattern.compile("discrete-input:" + ModbusField.ADDRESS_PATTERN);
+public static final Pattern ADDRESS_SHORTER_PATTERN = Pattern.compile("1" 
+ ModbusField.ADDRESS_PATTERN);
+public static final Pattern ADDRESS_SHORT_PATTERN = Pattern.compile("1x" + 
ModbusField.ADDRESS_PATTERN);
 
 public ModbusFieldDiscreteInput(int address, Integer quantity) {
 super(address, quantity);
 }
 
-public static ModbusFieldDiscreteInput of(String addressString) throws 
PlcInvalidFieldException {
-Matcher matcher = ADDRESS_PATTERN.matcher(addressString);
-if (!matcher.matches()) {
-throw new PlcInvalidFieldException(addressString, ADDRESS_PATTERN);
+public static boolean matches(String addressString) {
+return ADDRESS_PATTERN.matcher(addressString).matches() ||
+ADDRESS_SHORTER_PATTERN.matcher(addressString).matches() ||
+ADDRESS_SHORT_PATTERN.matcher(addressString).matches();
+}
+
+public static Matcher getMatcher(String addressString) throws 
PlcInvalidFieldException {
+Matcher matcher;
+if (ADDRESS_PATTERN.matcher(addressString).matches()) {
+  matcher = ADDRESS_PATTERN.matcher(addressString);
+} else if (ADDRESS_SHORT_PATTERN.matcher(addressString).matches()) {
+  matcher = ADDRESS_SHORT_PATTERN.matcher(addressString);
+} else if (ADDRESS_SHORTER_PATTERN.matcher(addressString).matches()) {
+  matcher = ADDRESS_SHORTER_PATTERN.matcher(addressString);
+} else {
+  throw new PlcInvalidFieldException(addressString, ADDRESS_PATTERN);
 }
-int address = Integer.parseInt(matcher.group("address"));
+return matcher;
+}
+
+public static ModbusFieldDiscreteInput of(String addressString) throws 
PlcInvalidFieldException {
+Matcher matcher = getMatcher(addressString);
+matcher.find();

Review comment:
   Calling the getMatcher mehod checks for the case that the pattern isn't 
found by raising the exception. I moved the exception to that method so I 
didn't have to worry about returning a null value.
   
   I found that the find() call then needs to be added as I wasn't calling the 
matches() method to find the first match. We can ignore the result of this as 
the getMatcher method has already confirmed that at least one instance will be 
found.
   





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [plc4x] chrisdutz commented on a change in pull request #172: Feature/modbus add additional address formats and change lowest register to 1.

2020-07-17 Thread GitBox


chrisdutz commented on a change in pull request #172:
URL: https://github.com/apache/plc4x/pull/172#discussion_r456321362



##
File path: 
plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/field/ModbusFieldDiscreteInput.java
##
@@ -26,21 +26,40 @@ Licensed to the Apache Software Foundation (ASF) under one
 public class ModbusFieldDiscreteInput extends ModbusField {
 
 public static final Pattern ADDRESS_PATTERN = 
Pattern.compile("discrete-input:" + ModbusField.ADDRESS_PATTERN);
+public static final Pattern ADDRESS_SHORTER_PATTERN = Pattern.compile("1" 
+ ModbusField.ADDRESS_PATTERN);
+public static final Pattern ADDRESS_SHORT_PATTERN = Pattern.compile("1x" + 
ModbusField.ADDRESS_PATTERN);
 
 public ModbusFieldDiscreteInput(int address, Integer quantity) {
 super(address, quantity);
 }
 
-public static ModbusFieldDiscreteInput of(String addressString) throws 
PlcInvalidFieldException {
-Matcher matcher = ADDRESS_PATTERN.matcher(addressString);
-if (!matcher.matches()) {
-throw new PlcInvalidFieldException(addressString, ADDRESS_PATTERN);
+public static boolean matches(String addressString) {
+return ADDRESS_PATTERN.matcher(addressString).matches() ||
+ADDRESS_SHORTER_PATTERN.matcher(addressString).matches() ||
+ADDRESS_SHORT_PATTERN.matcher(addressString).matches();
+}
+
+public static Matcher getMatcher(String addressString) throws 
PlcInvalidFieldException {
+Matcher matcher;
+if (ADDRESS_PATTERN.matcher(addressString).matches()) {
+  matcher = ADDRESS_PATTERN.matcher(addressString);
+} else if (ADDRESS_SHORT_PATTERN.matcher(addressString).matches()) {
+  matcher = ADDRESS_SHORT_PATTERN.matcher(addressString);
+} else if (ADDRESS_SHORTER_PATTERN.matcher(addressString).matches()) {
+  matcher = ADDRESS_SHORTER_PATTERN.matcher(addressString);
+} else {
+  throw new PlcInvalidFieldException(addressString, ADDRESS_PATTERN);
 }
-int address = Integer.parseInt(matcher.group("address"));
+return matcher;
+}
+
+public static ModbusFieldDiscreteInput of(String addressString) throws 
PlcInvalidFieldException {
+Matcher matcher = getMatcher(addressString);
+matcher.find();

Review comment:
   Yeah .. in that case I would prefer "5 or more" digits (You can probably 
just do 5-6 digits)





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [plc4x] hutcheb commented on a change in pull request #172: Feature/modbus add additional address formats and change lowest register to 1.

2020-07-17 Thread GitBox


hutcheb commented on a change in pull request #172:
URL: https://github.com/apache/plc4x/pull/172#discussion_r456318413



##
File path: 
plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/field/ModbusFieldCoil.java
##
@@ -26,17 +26,37 @@ Licensed to the Apache Software Foundation (ASF) under one
 public class ModbusFieldCoil extends ModbusField {
 
 public static final Pattern ADDRESS_PATTERN = Pattern.compile("coil:" + 
ModbusField.ADDRESS_PATTERN);
+public static final Pattern ADDRESS_SHORTER_PATTERN = Pattern.compile("0" 
+ ModbusField.ADDRESS_PATTERN);

Review comment:
   I wasn't too sure about this because depending on the device they will 
list the Modbus address as 40001 or 41. The extra 0 seems to have been 
added when the larger memory areas started to be used.
   
   In Proworx NXT when referencing Modbus addresses you can just type 41 and it 
will always take the first digit as the memory area, the rest will be the 
address. I wasn't sure about extending the format to this extent.
   
   It doesn't make sense to have more than 5 digits for the address though or 
values greater than 65536. I can fix this up.





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [plc4x] chrisdutz commented on a change in pull request #172: Feature/modbus add additional address formats and change lowest register to 1.

2020-07-17 Thread GitBox


chrisdutz commented on a change in pull request #172:
URL: https://github.com/apache/plc4x/pull/172#discussion_r456315294



##
File path: 
plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/field/ModbusField.java
##
@@ -28,33 +28,33 @@ Licensed to the Apache Software Foundation (ASF) under one
 public abstract class ModbusField implements PlcField {
 
 public static final Pattern ADDRESS_PATTERN = 
Pattern.compile("(?\\d+)(\\[(?\\d+)])?");
+protected static final int protocolAddressOffset = 1;
 
 private final int address;
 
 private final int quantity;
 
 public static ModbusField of(String addressString) throws 
PlcInvalidFieldException {
-Matcher matcher = 
ModbusFieldCoil.ADDRESS_PATTERN.matcher(addressString);
-if(matcher.matches()) {
+if(ModbusFieldCoil.matches(addressString)) {
 return ModbusFieldCoil.of(addressString);
 }
-matcher = 
ModbusFieldDiscreteInput.ADDRESS_PATTERN.matcher(addressString);
-if(matcher.matches()) {
+if(ModbusFieldDiscreteInput.matches(addressString)) {
 return ModbusFieldDiscreteInput.of(addressString);
 }
-matcher = 
ModbusFieldHoldingRegister.ADDRESS_PATTERN.matcher(addressString);
-if(matcher.matches()) {
+if(ModbusFieldHoldingRegister.matches(addressString)) {
 return ModbusFieldHoldingRegister.of(addressString);
 }
-matcher = 
ModbusFieldInputRegister.ADDRESS_PATTERN.matcher(addressString);
-if(matcher.matches()) {
+if(ModbusFieldInputRegister.matches(addressString)) {
 return ModbusFieldInputRegister.of(addressString);
 }
 throw new PlcInvalidFieldException("Unable to parse address: " + 
addressString);
 }
 
 protected ModbusField(int address, Integer quantity) {
 this.address = address;
+if ((this.address + protocolAddressOffset) <= 0) {

Review comment:
   Aaaahh yeah ... now I got you ... and yes ... indeed "address" would be 
the decremented value ... so yeah ... just ignore my comment ;-)





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [plc4x] hutcheb commented on a change in pull request #172: Feature/modbus add additional address formats and change lowest register to 1.

2020-07-17 Thread GitBox


hutcheb commented on a change in pull request #172:
URL: https://github.com/apache/plc4x/pull/172#discussion_r456313688



##
File path: 
plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/field/ModbusField.java
##
@@ -28,33 +28,33 @@ Licensed to the Apache Software Foundation (ASF) under one
 public abstract class ModbusField implements PlcField {
 
 public static final Pattern ADDRESS_PATTERN = 
Pattern.compile("(?\\d+)(\\[(?\\d+)])?");
+protected static final int protocolAddressOffset = 1;
 
 private final int address;
 
 private final int quantity;
 
 public static ModbusField of(String addressString) throws 
PlcInvalidFieldException {
-Matcher matcher = 
ModbusFieldCoil.ADDRESS_PATTERN.matcher(addressString);
-if(matcher.matches()) {
+if(ModbusFieldCoil.matches(addressString)) {
 return ModbusFieldCoil.of(addressString);
 }
-matcher = 
ModbusFieldDiscreteInput.ADDRESS_PATTERN.matcher(addressString);
-if(matcher.matches()) {
+if(ModbusFieldDiscreteInput.matches(addressString)) {
 return ModbusFieldDiscreteInput.of(addressString);
 }
-matcher = 
ModbusFieldHoldingRegister.ADDRESS_PATTERN.matcher(addressString);
-if(matcher.matches()) {
+if(ModbusFieldHoldingRegister.matches(addressString)) {
 return ModbusFieldHoldingRegister.of(addressString);
 }
-matcher = 
ModbusFieldInputRegister.ADDRESS_PATTERN.matcher(addressString);
-if(matcher.matches()) {
+if(ModbusFieldInputRegister.matches(addressString)) {
 return ModbusFieldInputRegister.of(addressString);
 }
 throw new PlcInvalidFieldException("Unable to parse address: " + 
addressString);
 }
 
 protected ModbusField(int address, Integer quantity) {
 this.address = address;
+if ((this.address + protocolAddressOffset) <= 0) {

Review comment:
   this.address stores the address sent over the wire starting at 0 instead 
of address 1. I didn't want to have a check that checks if the value is < 0 and 
have the exception say you can't have an address < 1 as it might be confusing.
   
   In the constructor for each memory area I subtract 1 from the address before 
storing it in this.address.





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [plc4x] chrisdutz edited a comment on pull request #170: PLC4X-207 When a Handler Timeout occurs cancel the read future to not…

2020-07-17 Thread GitBox


chrisdutz edited a comment on pull request #170:
URL: https://github.com/apache/plc4x/pull/170#issuecomment-659973093


   > @ottobackwards I agree with you (I also would like to have a test), yet 
I'm unsure how we can provide a Test with lower complexity then the 
implementation. Perhaps @chrisdutz has an idea with his setup and the test 
suite?
   
   Hi ... I think you should have a look at the XML-base IT framework ... here 
I can add delays which would be a perfect match for implementing such a test.
   
   Please check out:
   plc4j/drivers/s7/src/test/resources/testsuite/S7DriverIT.xml
   Where I would also suggest to add the test to.



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [plc4x] chrisdutz commented on pull request #170: PLC4X-207 When a Handler Timeout occurs cancel the read future to not…

2020-07-17 Thread GitBox


chrisdutz commented on pull request #170:
URL: https://github.com/apache/plc4x/pull/170#issuecomment-659973093


   > @ottobackwards I agree with you (I also would like to have a test), yet 
I'm unsure how we can provide a Test with lower complexity then the 
implementation. Perhaps @chrisdutz has an idea with his setup and the test 
suite?
   
   Hi ... I think you should have a look at the XML-base IT framework ... here 
I can add delays which would be a perfect match for implementing such a test.



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [plc4x] chrisdutz merged pull request #166: Team addition strljic

2020-07-17 Thread GitBox


chrisdutz merged pull request #166:
URL: https://github.com/apache/plc4x/pull/166


   



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [plc4x] chrisdutz commented on pull request #171: [PLC4X-216]update IoTDB JDBC example and session API example; add the related doc on website

2020-07-17 Thread GitBox


chrisdutz commented on pull request #171:
URL: https://github.com/apache/plc4x/pull/171#issuecomment-659969892


   Probably might be a good idea to add the "useJDBC" to the options and allow 
switching from the command-line?



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




Simple way to review PRs in IntelliJ

2020-07-17 Thread Christofer Dutz
Hi all,

I just tried out something new for me.
IntelliJ seems to have some functionality for working with Github PullRequests.

https://blog.jetbrains.com/idea/2018/10/intellij-idea-2018-3-eap-github-pull-requests-and-more/

This makes it super-easy to create, and work on github PRs but also to review 
them.
This was always a little complicated to try out locally, with this tooling it’s 
super-easy.

Just thought I’d share.

Chris


Re: [jira] [Created] (PLC4X-214) [Modbus] Holding register addresses have an offset of 1 (Not reading the correct address)

2020-07-17 Thread Christofer Dutz
Hi Ben, 

I just reviewed your changes and I like them a lot ... I did find some minor 
things that might deserve tweaking.
But thanks for that :)

Chris

Am 17.07.20, 05:14 schrieb "Ben Hutcheson" :

Hi,

Sure I'll give it a shot,

I have created a pull request to be able to use the (01, 0x1,
11, etc..)  address formats as well as to change the minimum address to
1. Corresponding to 01 or the first coil.
Please don't hold back on criticizing it, it is the only way I'll learn.

I'll take a look at adding the extended memory area next, I'm expecting it
will take me around a week.

Ben


On Thu, Jul 16, 2020 at 12:25 PM Christofer Dutz 
wrote:

> Aaaahh ... perfect
>
> Thanks Ben and Niclas.
> Guess I'll be doing some Modbus coding pretty soon :-)
>
> But if someone wants to try I'd be more than happy :)
>
> Chris
>
>
>
> Am 16.07.20, 11:48 schrieb "Ben Hutcheson" :
>
> Hi,
>
> *I think we have 6 separate memory areas. Do you have a mapping, That 
I
> could use? I mean which first digit represents which memory area?*
> I thought there were only 5 areas?
> 0x - Coils
> 1x - Inputs
> 3x - Input Registers
> 4x - Holding Registers
> 6x - Extended Registers
>
> I've seen the IEEE format for 32-bit floats, also another format that
> gets
> used is multiplying the float by 100 or 1000 and then dividing it by
> the
> same on the other end. e.g. 56.67 becomes 5667,
>
> Kind Regards
>
> Ben
>
> On Thu, Jul 16, 2020 at 4:10 AM Niclas Hedhman 
> wrote:
>
> > For floats, I have only seen IEEE format. But can't rule out other.
> >
> > On Thu, Jul 16, 2020, 14:57 Christofer Dutz <
> christofer.d...@c-ware.de>
> > wrote:
> >
> > > Guess it should be possible for plc4x to interpret INT as two
> shorts long
> > > as four... In that case it could probably also handle half
> precision
> > floats
> > > (16 bit), full floats and double, if the encoding is somewhat
> standard
> > > (which I assume it's not)
> > >
> > > Chris
> > > 
> > > Von: Niclas Hedhman 
> > > Gesendet: Donnerstag, 16. Juli 2020 08:33
> > > An: dev@plc4x.apache.org 
> > > Betreff: Re: [jira] [Created] (PLC4X-214) [Modbus] Holding 
register
> > > addresses have an offset of 1 (Not reading the correct address)
> > >
> > > To make things worse, there is equipment on the market with both
> 32-bit
> > > numbers as well IEEE floats.
> > >
> > > And many clients are incapable of doing something meaningful with
> > those...
> > >
> > >
> > > And then there is equipment that uses one register to indicate the
> > > magnitude of one or more other registers, say 1="divide by 1",
> 2="divide
> > by
> > > 10"...
> > >
> > >
> > >
> > > Niclas
> > >
> > > On Thu, Jul 16, 2020, 14:28 Christofer Dutz <
> christofer.d...@c-ware.de>
> > > wrote:
> > >
> > > > Hi Ben and Otto,
> > > >
> > > > First off all, thank you Ben for that very detailed explanation.
> It
> > does
> > > > seem as if we should extend the parser to support the different
> numeric
> > > > variants. I don't see any problems in supporting both the
> hex-like one
> > as
> > > > well as the pure numeric one.
> > > >
> > > > I think we have 6 separate memory areas. Do you have a mapping,
> That I
> > > > could use? I mean which first digit represents which memory 
area?
> > > >
> > > > @otto Modbus doesn't allow floats. Just bits (coils) and shorts
> > > > (registers)... Haven't seen a somewhat standard way to encode
> anything
> > > else.
> > > >
> > > > Chris
> > > > 
> > > > Von: Otto Fowler 
> > > > Gesendet: Donnerstag, 16. Juli 2020 06:41
> > > > An: dev@plc4x.apache.org 
> > > > Betreff: Re: [jira] [Created] (PLC4X-214) [Modbus] Holding
> register
> > > > addresses have an offset of 1 (Not reading the correct address)
> > > >
> > > > Don’t forget embedded protocols are possible,
> > > > different devices format floats differently
> > > > some devices don’t want persistent connections
> > > > etc etc
> > > >
> > > > On July 15, 2020 at 20:48:39, Ben Hutcheson (
> ben.hut...@gmail.com)
> > > wrote:
> > > >
> > > > Hi,
> > > >
> > > > Answering some of the 

[GitHub] [plc4x] chrisdutz commented on a change in pull request #172: Feature/modbus add additional address formats and change lowest register to 1.

2020-07-17 Thread GitBox


chrisdutz commented on a change in pull request #172:
URL: https://github.com/apache/plc4x/pull/172#discussion_r456296593



##
File path: 
plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/field/ModbusFieldDiscreteInput.java
##
@@ -26,21 +26,40 @@ Licensed to the Apache Software Foundation (ASF) under one
 public class ModbusFieldDiscreteInput extends ModbusField {
 
 public static final Pattern ADDRESS_PATTERN = 
Pattern.compile("discrete-input:" + ModbusField.ADDRESS_PATTERN);
+public static final Pattern ADDRESS_SHORTER_PATTERN = Pattern.compile("1" 
+ ModbusField.ADDRESS_PATTERN);
+public static final Pattern ADDRESS_SHORT_PATTERN = Pattern.compile("1x" + 
ModbusField.ADDRESS_PATTERN);
 
 public ModbusFieldDiscreteInput(int address, Integer quantity) {
 super(address, quantity);
 }
 
-public static ModbusFieldDiscreteInput of(String addressString) throws 
PlcInvalidFieldException {
-Matcher matcher = ADDRESS_PATTERN.matcher(addressString);
-if (!matcher.matches()) {
-throw new PlcInvalidFieldException(addressString, ADDRESS_PATTERN);
+public static boolean matches(String addressString) {
+return ADDRESS_PATTERN.matcher(addressString).matches() ||
+ADDRESS_SHORTER_PATTERN.matcher(addressString).matches() ||
+ADDRESS_SHORT_PATTERN.matcher(addressString).matches();
+}
+
+public static Matcher getMatcher(String addressString) throws 
PlcInvalidFieldException {
+Matcher matcher;
+if (ADDRESS_PATTERN.matcher(addressString).matches()) {
+  matcher = ADDRESS_PATTERN.matcher(addressString);
+} else if (ADDRESS_SHORT_PATTERN.matcher(addressString).matches()) {
+  matcher = ADDRESS_SHORT_PATTERN.matcher(addressString);
+} else if (ADDRESS_SHORTER_PATTERN.matcher(addressString).matches()) {
+  matcher = ADDRESS_SHORTER_PATTERN.matcher(addressString);
+} else {
+  throw new PlcInvalidFieldException(addressString, ADDRESS_PATTERN);
 }
-int address = Integer.parseInt(matcher.group("address"));
+return matcher;
+}
+
+public static ModbusFieldDiscreteInput of(String addressString) throws 
PlcInvalidFieldException {
+Matcher matcher = getMatcher(addressString);
+matcher.find();

Review comment:
   You are ignoring the result of the find ... what's it meant for?

##
File path: 
plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/field/ModbusField.java
##
@@ -28,33 +28,33 @@ Licensed to the Apache Software Foundation (ASF) under one
 public abstract class ModbusField implements PlcField {
 
 public static final Pattern ADDRESS_PATTERN = 
Pattern.compile("(?\\d+)(\\[(?\\d+)])?");
+protected static final int protocolAddressOffset = 1;
 
 private final int address;
 
 private final int quantity;
 
 public static ModbusField of(String addressString) throws 
PlcInvalidFieldException {
-Matcher matcher = 
ModbusFieldCoil.ADDRESS_PATTERN.matcher(addressString);
-if(matcher.matches()) {
+if(ModbusFieldCoil.matches(addressString)) {
 return ModbusFieldCoil.of(addressString);
 }
-matcher = 
ModbusFieldDiscreteInput.ADDRESS_PATTERN.matcher(addressString);
-if(matcher.matches()) {
+if(ModbusFieldDiscreteInput.matches(addressString)) {
 return ModbusFieldDiscreteInput.of(addressString);
 }
-matcher = 
ModbusFieldHoldingRegister.ADDRESS_PATTERN.matcher(addressString);
-if(matcher.matches()) {
+if(ModbusFieldHoldingRegister.matches(addressString)) {
 return ModbusFieldHoldingRegister.of(addressString);
 }
-matcher = 
ModbusFieldInputRegister.ADDRESS_PATTERN.matcher(addressString);
-if(matcher.matches()) {
+if(ModbusFieldInputRegister.matches(addressString)) {
 return ModbusFieldInputRegister.of(addressString);
 }
 throw new PlcInvalidFieldException("Unable to parse address: " + 
addressString);
 }
 
 protected ModbusField(int address, Integer quantity) {
 this.address = address;
+if ((this.address + protocolAddressOffset) <= 0) {
+throw new IllegalArgumentException("address must be greater then 
zero. Was " + (this.address + protocolAddressOffset));

Review comment:
   Same as above

##
File path: 
plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/field/ModbusField.java
##
@@ -28,33 +28,33 @@ Licensed to the Apache Software Foundation (ASF) under one
 public abstract class ModbusField implements PlcField {
 
 public static final Pattern ADDRESS_PATTERN = 
Pattern.compile("(?\\d+)(\\[(?\\d+)])?");
+protected static final int protocolAddressOffset = 1;
 
 private final int address;
 
 private final int quantity;
 
 public static ModbusField of(String addressString)