Skip to content

Feature/add mikrotik device tracker#7366

Merged
balloob merged 4 commits into
home-assistant:devfrom
LvivEchoes:feature/add-mikrotik-device-tracker
Apr 30, 2017
Merged

Feature/add mikrotik device tracker#7366
balloob merged 4 commits into
home-assistant:devfrom
LvivEchoes:feature/add-mikrotik-device-tracker

Conversation

@LvivEchoes
Copy link
Copy Markdown
Contributor

Description:

Add Mikrotik routers support as device tracker

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#2522

Example entry for configuration.yaml (if applicable):

device_tracker:
  - platform: mikrotik
    host: 192.168.81.1
    username: admin
    password: ilovevictoria

Checklist:

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

@mention-bot
Copy link
Copy Markdown

@LvivEchoes, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @fabaff and @robbiet480 to be potential reviewers.

@balloob balloob merged commit e4ebae5 into home-assistant:dev Apr 30, 2017
@balloob
Copy link
Copy Markdown
Member

balloob commented Apr 30, 2017

Thanks!

@balloob balloob mentioned this pull request May 5, 2017
@cyberplant
Copy link
Copy Markdown
Contributor

cyberplant commented May 20, 2017

Hello there!

I found this and been trying to use it for the last week. Today I decided to check the code and found out that I was checking the wrong PR (the one that was using tikapy #5830 ), and this only relies on wireless clients.

As in my case my router doesn't have wireless capabilities, I want to use only the DHCP output.

I quick-patched the module on my computer and it's working, but want to ask: is it OK if I submit a PR for this? or you prefer to keep it just wireless?

@LvivEchoes
Copy link
Copy Markdown
Contributor Author

LvivEchoes commented May 21, 2017 via email

@LvivEchoes
Copy link
Copy Markdown
Contributor Author

LvivEchoes commented May 21, 2017 via email

@cyberplant
Copy link
Copy Markdown
Contributor

Hello @LvivEchoes ! Thanks for your new PR.

I've configured lease time on 10 minutes, so it's not dramatic. But, one thing I did on my version was to filter the dhcp-table if "active-address" is present. I don't know if that field is present on all the routers, but on my case it allows me to have static dhcp leases and only 'detect presence' when the device is alive.

I did something like this:

            self.last_results = {
                device.get('mac-address'):
                    mac_names.get(device.get('mac-address'))
                for device in wireless_clients
            } or {
                device.get('mac-address'):
                    mac_names.get(device.get('mac-address'))
                for device in device_names
                if device.get('active-address')
            }

If this active-address is present on most routers, I think we can use it to improve the detection, what do you think?

@LvivEchoes
Copy link
Copy Markdown
Contributor Author

@cyberplant nice catch! I should test it in my env and if all is ok, I create PR with this improvement!

@cyberplant
Copy link
Copy Markdown
Contributor

Perfect, thanks @LvivEchoes. I've been using it at home and it's working fine.

pvizeli pushed a commit that referenced this pull request Jun 20, 2017
We only want to know which of the DHCP clients are indeed active.

For example: I've a table of static DHCP leases with most of the IPs of my network, so this module is always detecting them as present. With my patch only the active ones will be detected as present.

I already mentioned here: #7366 (comment)
dethpickle pushed a commit to dethpickle/home-assistant that referenced this pull request Aug 18, 2017
We only want to know which of the DHCP clients are indeed active.

For example: I've a table of static DHCP leases with most of the IPs of my network, so this module is always detecting them as present. With my patch only the active ones will be detected as present.

I already mentioned here: home-assistant#7366 (comment)
@home-assistant home-assistant locked and limited conversation to collaborators Sep 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants