Feature/cdutz/go ads ng (Streamlining of PLC4X API in PLC4Go and PLC4J) #576
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…specification. - renamed the property "len" to "strLen" to avoid problems in Go
First working version that correctly sends out the requests to all valid IPs and then correctly processes the responses
- Switched to using a pre-allocated buffer
- Made the buffer allocation actually work nicely.
…ds-ng # Conflicts: # plc4go/internal/ads/Field.go # plc4go/internal/ads/Reader.go # plc4go/internal/ads/Writer.go
…pecification. - Got the parsing of data-type and symbol-table working - Tweaked the way the endianess is handles.
- PlcBrowseItem's getChildren is now a Map
- PlcBrowseRequest now has a executeWithInterceptor method
- PlcBrowseResponse now supports multiple queries and therefore has:
- a getQueryNames() method
- the getResponseCode method was replaced with a getResponseCode(queryName) method
- the getValues() method was replaced with a getValues(queryName) method
- PlcField:
- Removed the getDefaultJavaType() as PlcValueType provides this
- replaced String getPlcDataType() with PlcValueType getPlcValueType()
- replaced int getNumberOfElements() with List<ArrayInfo> getArrayInfo()
- A new type PlcQuery for being the base of Browse queries
- PlcValueType now provides a default java type for each constant
Protocols:
- Extended the data-types of the following protocols to allow mapping to the PlcValueTypes of the API module:
- canopen
- genericcan
Adjusted the rest of the codebase accordingly.
- Continued porting the PLC4Go API to be on par with the updated PLC4J API - Started porting most of the existing code, but espeically the Browse parts still need quite some work.
- Finished implementing the multi-level-array-info-parsing
- Continued cleaning up the code after the changes.
- Continued cleaning up the code after the changes.
- Continued cleaning up the code after the changes.
- Finished cleaning the compilation up next will be getting the tests back green
- Finished fixing the go tests.
…" or the "with-all-tests" profiles. - Got the "all-tests" profile working again.
chrisdutz
requested review from
sruehl and
hutcheb
and removed request for
sruehl
sruehl
requested changes
Nov 7, 2022
plc4go/internal/ads/Browser.go
Outdated
| } | ||
|
|
||
| /* | ||
| func LALALA(){ |
| if symbol, ok := m.symbolTable[symbolName]; !ok { | ||
| // Couldn't find the base symbol | ||
| return nil | ||
| } else if len(remainingSegments) == 0 { |
There was a problem hiding this comment.
else is unnecessary here as the if above has a return
| } else if len(remainingSegments) == 0 { | ||
| // TODO: Convert the symbol itself into a PlcBrowseField | ||
| return nil | ||
| } else { |
There was a problem hiding this comment.
else is unnecessary here as the if above has a return
plc4go/internal/ads/Browser.go
Outdated
| if symbolDataType, ok := m.dataTypeTable[symbolDataTypeName]; !ok { | ||
| // Couldn't find data type | ||
| return nil | ||
| } else { |
There was a problem hiding this comment.
else is unnecessary here as the if above has a return
| } | ||
| } | ||
| } | ||
| // TODO: Couldn't find property with the given name. |
There was a problem hiding this comment.
TODO: is that meant to be fixed?
| if needsResolving(field) { | ||
| adsField, err := castToSymbolicPlcFieldFromPlcField(field) | ||
| if err != nil { | ||
| /* // TODO: handle context |
plc4go/internal/cbus/Field.go
Outdated
| @@ -44,9 +47,16 @@ func (s StatusRequestType) String() string { | |||
| return "" | |||
| } | |||
|
|
|||
| type CbusField interface { | |||
There was a problem hiding this comment.
I would just name that Field here...
plc4go/spi/default/DefaultBrowser.go
Outdated
| apiModel "github.com/apache/plc4x/plc4go/pkg/api/model" | ||
| "github.com/apache/plc4x/plc4go/spi" | ||
| "github.com/apache/plc4x/plc4go/spi/model" | ||
| ) | ||
|
|
||
| // DefaultBrowserRequirements adds required methods to Browser that are needed when using DefaultBrowser | ||
| type DefaultBrowserRequirements interface { | ||
| BrowseField(ctx context.Context, browseRequest apiModel.PlcBrowseRequest, interceptor func(result apiModel.PlcBrowseEvent) bool, fieldName string, field apiModel.PlcField) (apiModel.PlcResponseCode, []apiModel.PlcBrowseFoundField) | ||
| InternalBrowse(ctx context.Context, browseRequest apiModel.PlcBrowseRequest, interceptor func(result apiModel.PlcBrowseItem) bool, queryName string, query apiModel.PlcQuery) (apiModel.PlcResponseCode, []apiModel.PlcBrowseItem) |
There was a problem hiding this comment.
what was the reason to prefix that internal?
plc4go/spi/model/render_test.go
Outdated
| &DefaultResponse{}, | ||
| &DefaultPlcBrowseRequestResult{}, | ||
| &DefaultPlcBrowseRequest{}, | ||
| &DefaultPlcBrowseQueryResult{}, | ||
| //&DefaultPlcBrowseQueryResult{}, |
| @@ -127,7 +127,7 @@ func TestCombinations(t *testing.T) { | |||
| }, | |||
| }, | |||
| { | |||
| name: apiValues.LIST, | |||
| name: apiValues.List, | |||
There was a problem hiding this comment.
Why is the constant suddenly lower case?
…" or the "with-all-tests" profiles. - Got the RandomPackagesTest working again - Currently commented out the "0-termination" in the ReadBufferByteBased - Got the libpcapNg detection working on aarch64 mac
- Changed the ReadBufferByteBased to react on a system property "disable-string-0-termination" to make it work with the 0-terminating byte-buffer.
- Changed the ReadBufferByteBased to react on a system property "disable-string-0-termination" to make it work with the 0-terminating byte-buffer.
… bytes before the string -
… bytes before the string - Removed the disabling of 0-termination in the RandomPackageTest - Added some comments on the reading and writing of strings (expected format)
…ds-ng # Conflicts: # plc4go/protocols/bacnetip/readwrite/model/NLMDisconnectConnectionToNetwork.go # plc4go/protocols/bacnetip/readwrite/model/NLMEstablishConnectionToNetwork.go # plc4go/protocols/bacnetip/readwrite/model/NLMIAmRouterToNetwork.go # plc4go/protocols/bacnetip/readwrite/model/NLMICouldBeRouterToNetwork.go # plc4go/protocols/bacnetip/readwrite/model/NLMInitalizeRoutingTable.go # plc4go/protocols/bacnetip/readwrite/model/NLMInitalizeRoutingTableAck.go # plc4go/protocols/bacnetip/readwrite/model/NLMRejectRouterToNetwork.go # plc4go/protocols/bacnetip/readwrite/model/NLMRouterAvailableToNetwork.go # plc4go/protocols/bacnetip/readwrite/model/NLMRouterBusyToNetwork.go # plc4go/protocols/bacnetip/readwrite/model/NLMWhoIsRouterToNetwork.go
- Updated the builders to use the new addField and addFieldAddress naming
- Applied Sebasitan's feedback
- Applied Sebasitan's feedback
- Applied Sebasitan's feedback
- Renamed Field -> Tag in PLC4J
- Renamed Field -> Tag in PLC4J
- Renamed Field -> Tag in PLC4C
- Updated generated code for PLC4Go
- Removed getPlcValueType and getArrayInfo from the PlcBrowseItem as this information is in the Tag - Removed the PlcBrowseItemArrayInfo type, as this is replaced by ArrayInfo
- Continued the refactoring with PLC4Go
- Continued the refactoring with PLC4Go
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi all ... feedback highly appreciated