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

newdevice: Add support for KTI switches #5413

Merged
merged 3 commits into from Jan 17, 2017

Conversation

Projects
None yet
6 participants
@corny
Contributor

corny commented Jan 12, 2017

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

@mention-bot

This comment has been minimized.

mention-bot commented Jan 12, 2017

Thank you for submitting a PR @corny! We have found the following @laf, @murrant and @Rosiak based on the history of these files to review this PR.

@LibreNMS-CI

This comment has been minimized.

LibreNMS-CI commented Jan 12, 2017

Auto-Deploy finished, Test PR at http://5413.ci.librenms.org or https://5413.ci.librenms.org

@@ -0,0 +1 @@
1.3.6.1.2.1.1.1.0|4|KGS-1060 GigaBit Ethernet Switch

This comment has been minimized.

@murrant

murrant Jan 12, 2017

Member

We need both sysDescr and sysObjectId

This comment has been minimized.

@laf

laf Jan 12, 2017

Member

It's also worth switching the yaml discover to use sysObjectId if it's useful. Relying on sysDescr matches should be the last resort.

This comment has been minimized.

@corny

corny Jan 17, 2017

Contributor

The sysObjectId is not useful:

$ snmpbulkwalk -OntQX -v2c -c public $host 1.3.6.1.2.1.1
.1.3.6.1.2.1.1.1.0 = "KGS-1060 GigaBit Ethernet Switch"
.1.3.6.1.2.1.1.2.0 = .0.0

This comment has been minimized.

@laf

laf Jan 17, 2017

Member

Shoot that vendor

@@ -0,0 +1,4 @@
<?php
$hardware = trim(snmp_get($device, ".1.3.6.1.2.1.1.1.0", '-Ovq'), '"');

This comment has been minimized.

@murrant

murrant Jan 12, 2017

Member

sysDescr is already fetched, you can use $poll_device['sysDescr']

@LibreNMS-CI

This comment has been minimized.

LibreNMS-CI commented Jan 17, 2017

Auto-Deploy finished, Test PR at http://5413.ci.librenms.org or https://5413.ci.librenms.org

@@ -0,0 +1,4 @@
<?php
$hardware = trim(snmp_get($poll_device['sysDescr'], ".1.3.6.1.2.1.1.1.0", '-Ovq'), '"');

This comment has been minimized.

@laf

laf Jan 17, 2017

Member

What murrant meant here was that $poll_device['sysDescr'] already contains the output you want so you don't need another snmp call just $hardware = $poll_device['sysDescr']

@laf

This comment has been minimized.

Member

laf commented Jan 17, 2017

I've pushed the update.

@LibreNMS-CI

This comment has been minimized.

LibreNMS-CI commented Jan 17, 2017

Auto-Deploy finished, Test PR at http://5413.ci.librenms.org or https://5413.ci.librenms.org

@scrutinizer-notifier

This comment has been minimized.

scrutinizer-notifier commented Jan 17, 2017

The inspection completed: 1 updated code elements

@laf laf merged commit 84dca94 into librenms:master Jan 17, 2017

2 checks passed

Auto-Deploy Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@corny

This comment has been minimized.

Contributor

corny commented on 6a27212 Jan 17, 2017

Oh, sorry for the obvious mistake!

This comment has been minimized.

Member

laf replied Jan 18, 2017

It's no problem :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment