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

AeroHive Wireless Support #8500

Merged
merged 28 commits into from Apr 7, 2018

Conversation

Projects
None yet
4 participants
@theherodied
Contributor

theherodied commented Apr 3, 2018

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.

Testers

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

@theherodied

This comment has been minimized.

Contributor

theherodied commented Apr 3, 2018

@laf this isn't ready for review yet, I'm still working on it.

@theherodied theherodied referenced this pull request Apr 4, 2018

Closed

Add support for Aerohive Wireless #7791

5 of 5 tasks complete
$memory_oid = '1.3.6.1.4.1.26928.1.2.4.0';
$usage = snmp_get($device, $memory_oid, '-Ovq');
if (is_numeric($usage)) {
discover_mempool($valid_mempool, $device, '0', 'hiveos-wireless', 'Memory', '1', null, null);

This comment has been minimized.

@laf

laf Apr 4, 2018

Member

You don't need to pass '1', null, null. It's the defaults so you can just remove that.

* @package LibreNMS
* @link http://librenms.org
* @copyright 2018 Ryan Finney
* @author https://github.com/theherodied/

This comment has been minimized.

@laf

laf Apr 4, 2018

Member

May as well add Rosiak's name in your new block as well and remove the original copyright header. It's the same License but better to be in our format.

if (preg_match('/^(.+?),/', $device['sysDescr'], $hardware)) {
$hardware = $store[1];
}
$hardware = trim(snmp_get($device, '.1.3.6.1.4.1.4413.1.1.1.1.1.3.0', '-Ovq'), '"');
$version = trim(snmp_get($device, '.1.3.6.1.4.1.4413.1.1.1.1.1.13.0', '-Ovq'), '"');
$serial = trim(snmp_get($device, '.1.3.6.1.4.1.4413.1.1.1.1.1.4.0', '-Ovq'), '"');
$apmodel = trim(snmp_get($device, '.1.3.6.1.4.1.26928.1.2.6.0', '-OQv', '', ''), '" ');

This comment has been minimized.

@laf

laf Apr 4, 2018

Member

You can remove the blank '' from all of these, the defaults are null anyway.

$apmodel = trim(snmp_get($device, '.1.3.6.1.4.1.26928.1.2.6.0', '-OQv', '', ''), '" ');
if ($apmodel == "AP250" OR "AP550") {
// SNMPv2-SMI::enterprises.6530.11.1.0 = STRING: "2N IP Force"
$hardware = trim(snmp_get($device, '.1.3.6.1.4.1.26928.1.2.6.0', '-OQv', '', ''), '" ');

This comment has been minimized.

@laf

laf Apr 4, 2018

Member

Please update these three snmp_gets to a single snmp_get_multi_oid.

Does the output really have a quote then a space? " ? We already trim " from most snmp calls so you shouldn't need this.

This comment has been minimized.

@theherodied

theherodied Apr 4, 2018

Contributor

'Twas not needed.

@laf

Some style issues: https://travis-ci.org/librenms/librenms/jobs/362170038#L637

  • Can you add test data for os and wireless modules.
if (preg_match('/^(.+?),/', $device['sysDescr'], $hardware)) {
$hardware = $store[1];
}
$hardware = trim(snmp_get($device, '.1.3.6.1.4.1.4413.1.1.1.1.1.3.0', '-Ovq'), '"');
$version = trim(snmp_get($device, '.1.3.6.1.4.1.4413.1.1.1.1.1.13.0', '-Ovq'), '"');
$serial = trim(snmp_get($device, '.1.3.6.1.4.1.4413.1.1.1.1.1.4.0', '-Ovq'), '"');
$apmodel = trim(snmp_get($device, '.1.3.6.1.4.1.26928.1.2.6.0', '-OQv'), '"');

This comment has been minimized.

@laf

laf Apr 4, 2018

Member

Still no need to trim these as snmp_get does that already :)

$apmodel = trim(snmp_get($device, '.1.3.6.1.4.1.26928.1.2.6.0', '-OQv'), '"');
if ($apmodel == "AP250" OR "AP550") {
// AH-SYSTEM-MIB::.1.3.6.1.4.1.26928.1.2.6.0 = STRING: "AP250"
$hardware = trim(snmp_get($device, '.1.3.6.1.4.1.26928.1.2.6.0', '-OQv'), '"');

This comment has been minimized.

@laf

laf Apr 4, 2018

Member

Need to change these 3 snmp_gets to snmp_get_multi_oid :)

@laf laf added the Needs Tests 🦄 label Apr 4, 2018

theherodied added some commits Apr 4, 2018

@murrant

I see you are still working on the wireless name stuff. Don't forget to remove comments.

Also, when you are done you should add the json test data.

if (preg_match('/^(.+?),/', $device['sysDescr'], $hardware)) {
$hardware = $store[1];
}
$hardware = trim(snmp_get($device, '.1.3.6.1.4.1.4413.1.1.1.1.1.3.0', '-Ovq'), '"');
$version = trim(snmp_get($device, '.1.3.6.1.4.1.4413.1.1.1.1.1.13.0', '-Ovq'), '"');
$serial = trim(snmp_get($device, '.1.3.6.1.4.1.4413.1.1.1.1.1.4.0', '-Ovq'), '"');
$apmodel = snmp_get($device, '.1.3.6.1.4.1.26928.1.2.6.0', '-OQv');
if ($apmodel == "AP250" or "AP550") {

This comment has been minimized.

@murrant

murrant Apr 6, 2018

Member

This doesn't work how you think, it will always be true.

if ($apmodel == 'AP250' || $apmodel == 'AP550') {
$version2 = isset($data['.1.3.6.1.4.1.26928.1.2.12.0']) ? $data['.1.3.6.1.4.1.26928.1.2.12.0'] : '';
// Version has 'HiveOS ' included. We want to remove it so OS doesn't show HiveOS twice.
$prefix = 'HiveOS ';
$version = preg_replace('/^' . preg_quote($prefix, '/') . '/', '', $version2);

This comment has been minimized.

@murrant

murrant Apr 6, 2018

Member

You don't need to use preg_quote() here

Just use this regex '/^HiveOS /'

$apmodel = snmp_get($device, '.1.3.6.1.4.1.26928.1.2.6.0', '-OQv');
if ($apmodel == "AP250" or "AP550") {
// AeroHive AH-SYSTEM-MIB is broken so calling the OID.

This comment has been minimized.

@murrant

murrant Apr 6, 2018

Member

You've got a repaired version so you could use text oid again.

theherodied added some commits Apr 6, 2018

@scrutinizer-notifier

This comment has been minimized.

scrutinizer-notifier commented Apr 7, 2018

The inspection completed: 7 new issues, 7 updated code elements

@theherodied

This comment has been minimized.

Contributor

theherodied commented Apr 7, 2018

@laf and @murrant, I think this is good now. Let me know if I missed anything from the reviews.

@theherodied theherodied changed the title from AeroHive - Work In Progress to AeroHive Wireless support Apr 7, 2018

@theherodied theherodied changed the title from AeroHive Wireless support to AeroHive Wireless Support Apr 7, 2018

@laf

laf approved these changes Apr 7, 2018

LGTM

Changes have been made

@laf laf merged commit f694125 into librenms:master Apr 7, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@laf laf added Device 🖥 and removed Needs Tests 🦄 labels Apr 7, 2018

@theherodied theherodied deleted the theherodied:theherodied-aerohive-2 branch Apr 8, 2018

TheMysteriousX added a commit to TheMysteriousX/librenms that referenced this pull request May 20, 2018

device: Added AeroHive Wireless Support (librenms#8500)
* AeroHive 001

* AeroHive 002

* AeroHive 003

* Removed duplicate HiveOS for OS

* Added AP550 detection

* changed to Hiveos-Wireless

* changed back to HiveosWireless. Hiveoss-Wireless doesn't work

* changed back to HiveosWireless. Hiveoss-Wireless doesn't work

* Added Processor Usage.

* Added Mempool.

* working on review requests 001

* working on review requests 002

* stuck on radio stats

* WirelessSensor.php added channel 165

* Added AP250 snmprec

* Frequency work in progress.

* Frequency trying another way.

* Frequency trying another way 002

* Fixed AH-SMI-MIB? Added array try 003

* Frequency array is working thanks to Murrant.

* Frequency array description working.

* Added TX Power

* changed interface naming method

* Review changes 001

* Review changes 002

* Review changes 003

* Added json test data.

* Fixed test data 001.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 7, 2018

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