Re: Inquiry about the mspec
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
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?
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]'
BUILD-STABLE: Job 'PLC4X/PLC4X/develop [develop] [969]': Is back to normal.
[DISCUSS] How about naming "first time contributors" in RELEASE_NOTES?
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)
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]'
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]'
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.
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
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
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
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.
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.
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.
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.
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.
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.
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.
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…
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…
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
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
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
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)
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.
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)