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.

Sign up for GitHub

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

Jump to bottom

[tr064] Enhancements, code improvements and fixes #14468

Merged
merged 10 commits into from
Feb 24, 2023
Merged

[tr064] Enhancements, code improvements and fixes #14468

merged 10 commits into from
Feb 24, 2023

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented Feb 20, 2023 •
edited
Loading

@J-N-K
[tr064] Enhancements, code improvements and fixes
52d64f8 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K J-N-K added bug An unexpected problem or unintended behavior of an add-on enhancement An enhancement or new feature for an existing add-on labels Feb 20, 2023
J-N-K added 3 commits February 20, 2023 21:01
@J-N-K
address review comment
ee7113f 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K
address review comment
49ab41f 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K
address review comments
0c14605 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K J-N-K marked this pull request as ready for review February 20, 2023 20:26
@J-N-K J-N-K requested a review from a team as a code owner February 20, 2023 20:26
jlaur
jlaur requested changes Feb 20, 2023
View reviewed changes
Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

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

Thanks! I have added a few comments, mostly questions. Also, please regenerate the I18N properties file.

bundles/org.openhab.binding.tr064/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.tr064/src/main/resources/channels.xml Show resolved Hide resolved
bundles/org.openhab.binding.tr064/src/main/resources/channels.xml Outdated
<item type="Switch"/>
<service deviceType="urn:dslforum-org:device:LANDevice:1" serviceId="urn:LanDeviceHosts-com:serviceId:Hosts1"/>
<getAction name="GetSpecificHostEntry" argument="NewActive">
<parameter name="NewMACAddress" thingParameter="macOnline"
pattern="([0-9A-Fa-f]{2}:){5}[0-9A-Fa-f]{2}(\s*#.*)*"/>
</getAction>
</channel>
<channel name="macIP" label="MAC IP" description="IP of the device with the given MAC">
<channel name="macOnlineIpAddress" label="MAC Online IP" description="IP of the device with the given MAC"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a channel rename? The README has changed the description to "IP of the MAC (uses same parameter as macOnline)"

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. This channel was added to the other version before it was added here. That's why I renamed it.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, please remember to mention this in https://github.com/openhab/openhab-distro/blob/main/distributions/openhab/src/main/resources/bin/update.lst.

Should the description here be updated to be in line with the description in the README? In this PR this has changed from "IP of the device with the given MAC" to "IP of the MAC (uses same parameter as macOnline)", probably for a reason?

Copy link
Member Author

@J-N-K J-N-K Feb 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, there have been questions how to configure that channel, that's why I added a clarification.

I wonder if we should also rename the other channels that act on the same list of parameters (macSignalStrength1 and similar) for consistency.

J-N-K added 3 commits February 20, 2023 22:52
@J-N-K
revert accidentally deleted parts
f9bf866 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K
documentation fixes
c89a7c1 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K
codeowners
db2e042 
Signed-off-by: Jan N. Klug <github@klug.nrw>
J-N-K added 2 commits February 21, 2023 18:58
@J-N-K
improvements
0dac4cc 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K
regex
98886d4 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K
Copy link
Member Author

J-N-K commented Feb 23, 2023

I think everything is addressed now.

lsiepel
lsiepel approved these changes Feb 23, 2023
View reviewed changes
Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

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

LGTM

@lsiepel lsiepel mentioned this pull request Feb 23, 2023
Closed
@jlaur
Copy link
Contributor

jlaur commented Feb 24, 2023

I think everything is addressed now.

Thanks! Only thing I can see missing is I18N properties file regeneration.

@J-N-K
i18n
066a37c 
Signed-off-by: Jan N. Klug <github@klug.nrw>
jlaur
jlaur approved these changes Feb 24, 2023
View reviewed changes
Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

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

Thanks for backporting these improvements.

@jlaur jlaur merged commit cb31f42 into openhab:main Feb 24, 2023
@jlaur jlaur added this to the 4.0 milestone Feb 24, 2023
@J-N-K J-N-K deleted the tr064 branch February 25, 2023 18:59
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Feb 28, 2023
@J-N-K @nemerdaud
[tr064] Enhancements, code improvements and fixes (openhab#14468)
3f89a4b 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@t2000 t2000 mentioned this pull request Mar 11, 2023
Closed
@J-N-K J-N-K assigned J-N-K and unassigned J-N-K Mar 12, 2023
renescherer pushed a commit to renescherer/openhab-addons that referenced this pull request Mar 23, 2023
@J-N-K @renescherer
[tr064] Enhancements, code improvements and fixes (openhab#14468)
f7177a8 
Signed-off-by: Jan N. Klug <github@klug.nrw>
FordPrfkt pushed a commit to FordPrfkt/openhab-addons that referenced this pull request Apr 20, 2023
@J-N-K @FordPrfkt
[tr064] Enhancements, code improvements and fixes (openhab#14468)
cd31128 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K J-N-K mentioned this pull request Aug 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlaur jlaur jlaur approved these changes

@lsiepel lsiepel lsiepel approved these changes

Assignees
No one assigned
Labels
bug An unexpected problem or unintended behavior of an add-on enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Milestone
4.0
3 participants
@J-N-K @jlaur @lsiepel