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

newdevice: Added extrahop detection #6097

Merged
merged 4 commits into from
Mar 6, 2017
Merged

Conversation

geordish
Copy link
Contributor

@geordish geordish commented Mar 5, 2017

Fix for #6080

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.

  • Have you signed the Contributors agreement - please do NOT submit a pull request unless you have (signing the agreement in the same pull request is fine). Your commit message for signing the agreement must appear as per the docs.
  • Have you followed our code guidelines?

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

@mention-bot
Copy link

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

@LibreNMS-CI
Copy link

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

@laf laf changed the title Initial extrahop detection newdevice: Added extrahop detection Mar 5, 2017
@@ -12,6 +12,8 @@
} elseif (snmp_get($device, 'fwVersion.1', '-Osqnv', 'UBNT-AirFIBER-MIB', 'ubnt') !== false) {
$os = 'airos-af';
}
} elseif (snmp_get($device, 'EXTRAHOP-MIB::extrahopInfoVersionString', '-Osqnv') !== false) {
Copy link
Member

Choose a reason for hiding this comment

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

I know the other gets are like this but can you shift the EXTRAHOP-MIB to the end of the function:

snmp_get($device, 'extrahopInfoVersionString', '-Osqnv', 'EXTRAHOP-MIB')

* the source code distribution for details.
*/

$version = snmp_get($device, 'EXTRAHOP-MIB::extrahopInfoVersionString', '-Oqv');
Copy link
Member

Choose a reason for hiding this comment

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

snmp_get($device, 'extrahopInfoVersionString', '-Oqv', 'EXTRAHOP-MIB')

Copy link
Member

@laf laf left a comment

Choose a reason for hiding this comment

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

Can you move the MIB info mibs/extrahop

The html/images/os/extrahop.svg has part transparent background part white.

@geordish
Copy link
Contributor Author

geordish commented Mar 5, 2017

Don't think I can easily move the MIB, or the discovery doesn't work :(

@LibreNMS-CI
Copy link

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

@LibreNMS-CI
Copy link

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

@laf
Copy link
Member

laf commented Mar 6, 2017

For the MIB dir, you just need to specify the dir to look in for OS detection using the extra param in snmp_get() (any other detection will be fine as it will know the OS and therefore look in the correct dir).

@LibreNMS-CI
Copy link

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

@scrutinizer-notifier
Copy link

The inspection completed: 1 updated code elements

@laf laf merged commit 2a211e8 into librenms:master Mar 6, 2017
@geordish geordish deleted the issue-6080 branch March 6, 2017 14:47
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants