Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Fine tuning of the API and SPI #1786

Merged
merged 32 commits into from
Oct 2, 2024
Merged

Conversation

chrisdutz
Copy link
Contributor

What started as something to allow returning and handing INVALID_ADDRESS responses for individual items (Currently one address string, that the parser can't handle is enough to fail the entire request).

This required to add a getPlcResponseCode to the PlcTagRequests and to introduce a new set of items for the TagItem and TagValueItems, which allow returning a non-OK return code.

Alongside these changes, I started cleaning things up a little.

In the end I would like to bring back together the Tags and Queries and to only have Tags for both Browse and the other APIs. They could be QueryTags that allow filtering, but in general I think it would be better to only have one type for all APIs.

I could need a bit of help with the other languages.

…ponses for individual invalid addresses (Alongside a major cleanup)
@chrisdutz
Copy link
Contributor Author

Right now I would first update the test-case xml files for the current state and as soon as that's done, I would try to update the other languages accordingly.

@ottlukas ottlukas added API Application Programmable Interface of PLC4X feature labels Sep 25, 2024
@@ -37,8 +37,7 @@
*/
public class SymbolicAdsTag implements AdsTag {

// TODO: Model the end of this address to allow usage of multi-dimensional arrays.
private static final Pattern SYMBOLIC_ADDRESS_PATTERN = Pattern.compile("^(?<symbolicAddress>.+)");
private static final Pattern SYMBOLIC_ADDRESS_PATTERN = Pattern.compile("^([\\w_]+)(\"[\"\\d*]\")*(\\.(\\w+)(\"[\"\\d*]\")*)*");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you remove the symbolicAddress reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still working on this.... It's all still WIP.

@sruehl
Copy link
Contributor

sruehl commented Sep 25, 2024

@chrisdutz once you are in a happy state with this PR I can look into adding the Go-Variant for it

@chrisdutz
Copy link
Contributor Author

Thanks... I just thought I'd push my changes for feedback as early as possible. No need to waste time in something the rest dislikes.

@chrisdutz
Copy link
Contributor Author

So currently it seems that my changes had no effect on C, Python and all other languages, only Go probably needs some adjusting @sruehl would be cool, if you could have a lootk.

@chrisdutz chrisdutz marked this pull request as ready for review September 30, 2024 11:11
@chrisdutz chrisdutz merged commit 69b8bc1 into develop Oct 2, 2024
27 checks passed
@chrisdutz chrisdutz deleted the refactor/api-fine-tuning branch October 2, 2024 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Application Programmable Interface of PLC4X feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants