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

[mikrotik] Add PoE support #17351

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

[mikrotik] Add PoE support #17351

wants to merge 15 commits into from

Conversation

asp24
Copy link

@asp24 asp24 commented Aug 30, 2024

  • [Improvement] Implemented PoE monitoring and contol
  • [FIX] Crash on some routers with high available RAM available
  • [FIX] Modern WiFI impl packages registrations tables polling

@asp24 asp24 requested a review from duhast as a code owner August 30, 2024 14:48
@asp24 asp24 force-pushed the mikrotik-poe-support branch 2 times, most recently from 19df9ff to 2f78cb9 Compare September 2, 2024 16:12
@asp24
Fix integer overflow in RouterosSystemResources::get{Free,Total}Mem
261cd43 
Signed-off-by: asp24 <488588+asp24@users.noreply.github.com>
@asp24
Fix crash on devices with modern wifi packages
e93d20a 
Implemented polling Wireless and Capsman registrations tables from all known mikrotik  wifi impl

Signed-off-by: asp24 <488588+asp24@users.noreply.github.com>
@asp24
Refactored MikrotikBaseThingHandler::handleCommand() to early-return …
c7d3c61 
…style

Signed-off-by: asp24 <488588+asp24@users.noreply.github.com>
@asp24
Implement PoE monitoring and managing support
26a82fb 
Signed-off-by: asp24 <488588+asp24@users.noreply.github.com>
@asp24
Fix CS warnings
1471a2e 
Signed-off-by: asp24 <488588+asp24@users.noreply.github.com>
@asp24
Rename channel poeOut->poeOutState
fddecf4 
Signed-off-by: asp24 <488588+asp24@users.noreply.github.com>
@asp24
Add PoE-specific info to the documentation
dc31c0f 
Signed-off-by: asp24 <488588+asp24@users.noreply.github.com>
@lsiepel lsiepel added the enhancement An enhancement or new feature for an existing add-on label Sep 3, 2024
@lsiepel lsiepel changed the title [mikrotik] PoE support [mikrotik] Add PoE support Sep 3, 2024
@lsiepel
Copy link
Contributor

lsiepel commented Sep 3, 2024

Would it be possible to split the bugfix and the enhancement? If it is too much of a hassle its fine, but smaller/ dedicated PR.s are simpler to review, backport and also show up independend in the release notes

lsiepel
lsiepel requested changes Sep 9, 2024
View reviewed changes
Copy link
Contributor

@lsiepel lsiepel left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks. Besides the minor comments, it would be needed to provide thing upgrade instructions for the added channels.
For referece: https://www.openhab.org/docs/developer/bindings/thing-xml.html#updating-thing-types

Oh, when the labels are adjusted the i18n plugin has to be re-run to update the properties file.

...nding.mikrotik/src/main/java/org/openhab/binding/mikrotik/internal/model/RouterosDevice.java Show resolved Hide resolved
...nding.mikrotik/src/main/java/org/openhab/binding/mikrotik/internal/model/RouterosDevice.java Outdated Show resolved Hide resolved
...rotik/src/main/java/org/openhab/binding/mikrotik/internal/model/RouterosRouterboardInfo.java Show resolved Hide resolved
bundles/org.openhab.binding.mikrotik/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mikrotik/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mikrotik/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved
...c/main/java/org/openhab/binding/mikrotik/internal/handler/MikrotikInterfaceThingHandler.java Outdated Show resolved Hide resolved
asp24 and others added 6 commits September 16, 2024 20:34
@asp24 @lsiepel
Update bundles/org.openhab.binding.mikrotik/src/main/resources/OH-INF…
7939ceb 
…/thing/thing-types.xml

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Yuriy <asp24@users.noreply.github.com>
@asp24 @lsiepel
logger.error -> logger.warn on non-crit errors
b7c875d 
Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Yuriy <asp24@users.noreply.github.com>
@asp24 @lsiepel
logger.error -> logger.warn on non-crit errors
35b8115 
Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Yuriy <asp24@users.noreply.github.com>
@asp24
Fix typeId from poeOutState channel
cd1110c 
Signed-off-by: asp24 <488588+asp24@users.noreply.github.com>
@asp24
Labels should be < 23 chars, 2-3 words and capitalized
7b731c0 
Signed-off-by: asp24 <488588+asp24@users.noreply.github.com>
@asp24
Add instructions.xml for new channels
e832c67 
Signed-off-by: asp24 <488588+asp24@users.noreply.github.com>
lsiepel
lsiepel requested changes Sep 17, 2024
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.

Some additional comments. Also run the i18n plugin to generate the labels etc

bundles/org.openhab.binding.mikrotik/src/main/resources/OH-INF/update/instructions.xml
xsi:schemaLocation="https://openhab.org/schemas/update-description/v1.0.0 https://openhab.org/schemas/update-description-1.0.0.xsd">

<thing-type uid="mikrotik:routeros">
<instruction-set targetVersion="1">
Copy link
Contributor

Choose a reason for hiding this comment

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

The version also need to be added to the interface Thing as property.

<properties>
    <property name="thingTypeVersion">1</property>
  </properties>

Copy link
Contributor

Choose a reason for hiding this comment

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

Only this comment is left ^^, not sure if you missed it

bundles/org.openhab.binding.mikrotik/src/main/resources/OH-INF/update/instructions.xml Outdated Show resolved Hide resolved
asp24 and others added 2 commits September 22, 2024 12:03
@asp24 @lsiepel
Fix invalid thing-type uid in update/instructions.xml
8c28c6c 
Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Yuriy <asp24@users.noreply.github.com>
@asp24
i18n for mikrotik.properties
9cdd977 
Signed-off-by: asp24 <488588+asp24@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lsiepel lsiepel lsiepel requested changes

@duhast duhast Awaiting requested review from duhast duhast is a code owner

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@asp24 @lsiepel