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

New Device Support: FiberHome Switches S5800/S4800/S2800 #8569

Merged
merged 22 commits into from Apr 22, 2018

Conversation

Projects
None yet
3 participants
@czilian
Contributor

czilian commented Apr 17, 2018

Basic support / sensors / mempools / new logo SVG

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

czilian added some commits Apr 16, 2018

Fiber Home Switch Device Support
- Basic OS support
- New Logo
- Sensor support
- Fiberhome MIBS (wriSMI)
Fibrehome Switch Device Support
- add mempools discovery
Fiberhome Switch Device Support
- remove mib bases high/low limit for FAN entity
Fiberhome Switch Device Support
- remove unsupported processor high_limit option
FiberHome Switch Device Support
- snmpprec files for S28/S48/S58 added
- removed not used FH MIB files
Fiberhome Switch Device Support
- add snmprec for FHN5800 FHN4800 FHN2800
@czilian

This comment has been minimized.

Contributor

czilian commented Apr 17, 2018

Tried my luck... json for testing missing. Sorry...

@laf

I've left some inline comments.

Please delete the png file, you only need svg or png and we prefer svg.

For test data, you now need to run ./scripts/save-test-data.php

processors:
data:
-
oid: WRI-CPU-MIB::cpuTable

This comment has been minimized.

@laf

laf Apr 17, 2018

Member

All the MIBs you've specified need to be added at the top of this file like:

mib: WRI-CPU-MIB:WRI-TEMPERATURE-MIB:etc.... and removed from the oid lines.

This comment has been minimized.

@czilian

czilian Apr 18, 2018

Contributor

noted and done.

icon: fiberhome
discovery:
- sysObjectID:
- .1.3.6.1.4.1.3807.1.582815

This comment has been minimized.

@laf

laf Apr 17, 2018

Member

Maybe just drop these to one and use .1.3.6.1.4.1.3807.1. Our stats don't show this would pick up any other devices.

This comment has been minimized.

@czilian

czilian Apr 18, 2018

Contributor

noted and done.

- .1.3.6.1.4.1.3807.1.281802
- .1.3.6.1.4.1.3807.1.481005
over:
- { graph: device_bits, text: 'Device Traffic' }

This comment has been minimized.

@laf

laf Apr 17, 2018

Member

As you have sensors here you can also add them to this.

I.e:

includes/definitions/powerlogic.yaml:    - { graph: sensor_voltage, text: 'Voltage' }
includes/definitions/powerlogic.yaml:    - { graph: sensor_current, text: 'Current' }
includes/definitions/powerlogic.yaml:    - { graph: sensor_power, text: 'Power' }
includes/definitions/powerlogic.yaml:    - { graph: sensor_frequency, text: 'Frequency' }

This comment has been minimized.

@czilian

czilian Apr 18, 2018

Contributor

Yes I am aware. The only issue is that some variant devices don't have the sensors and then you get that ugly empty box. Is there a way to suppress that mouse over instruction for those?

- { graph: device_bits, text: 'Device Traffic' }
mib_dir:
- fiberhome
discovery_modules:

This comment has been minimized.

@laf

laf Apr 17, 2018

Member

All of these should be default so just remove the discovery and poller modules.

This comment has been minimized.

@czilian

czilian Apr 18, 2018

Contributor

noted and done.

/**
* LibreNMS - FiberHome Switch device support - mempools module
*
* @category Network_Monitoring

This comment has been minimized.

@laf

laf Apr 17, 2018

Member

Please use the copyright header we have in our docs:

https://docs.librenms.org/#Developing/Licensing/#copyright

This comment has been minimized.

@czilian

czilian Apr 18, 2018

Contributor

noted and done.

* the source code distribution for details.
**/
function hexbin($hex_string) // convert the hex OID content to character

This comment has been minimized.

@laf

laf Apr 17, 2018

Member

Please move this to includes/functions.php

This comment has been minimized.

@czilian

czilian Apr 18, 2018

Contributor

done

return $chr_string;
}
$sysDescrPieces = explode(" ", snmp_get($device, 'sysDescr.0', '-Oqv', 'SNMPv2-MIB')); //extract model from sysDescr

This comment has been minimized.

@laf

laf Apr 17, 2018

Member

We already have sysDescr in $device['sysDescr']

This comment has been minimized.

@czilian

czilian Apr 18, 2018

Contributor

done

$sysDescrPieces = explode(" ", snmp_get($device, 'sysDescr.0', '-Oqv', 'SNMPv2-MIB')); //extract model from sysDescr
$hwversion = str_replace(array("\r","\n"), '', snmp_get($device, 'msppDevHwVersion.0', '-Oqv', 'WRI-DEVICE-MIB'));

This comment has been minimized.

@laf

laf Apr 17, 2018

Member

Please convert these next two snmp_get() calls to snmp_get_multi_oid()

This comment has been minimized.

@czilian

czilian Apr 18, 2018

Contributor

done

@@ -0,0 +1,225 @@
WRI-CPU-MIB DEFINITIONS ::= BEGIN

This comment has been minimized.

@laf

laf Apr 17, 2018

Member

Please rename this mib file by dropping the .mib. Same for any other mibs.

This comment has been minimized.

@czilian

czilian Apr 18, 2018

Contributor

Not sure whether renaming the vendor MIB pays off in the long run. If there is an update from the vendor in future, need to always go through this renaming exercise again. The original MIB names are also declared inside the MIBs. But of cause it looks nicer not to have xx-MIB.MIB or small letters. Will rename for now to follow your standard.

@@ -0,0 +1,426 @@
--*****************************************************************************/

This comment has been minimized.

@laf

laf Apr 17, 2018

Member

This mib needs renaming to WRI-SMI

@czilian

This comment has been minimized.

Contributor

czilian commented Apr 17, 2018

Thx LAF. Will fix all this the next two days.

czilian added some commits Apr 18, 2018

FiberHome Switch Device Support
- adjusted code to PSR2 standard
@czilian

This comment has been minimized.

Contributor

czilian commented Apr 18, 2018

@laf Greetings, would appreciate if you can check this testing error. It gets stuck with another os. "fiberhome" - GPON from Alan because he is using the PNG logo. Hence, will stick to the old logo for now.

czilian and others added some commits Apr 19, 2018

@scrutinizer-notifier

This comment has been minimized.

scrutinizer-notifier commented Apr 22, 2018

The inspection completed: 1 updated code elements

@laf

laf approved these changes Apr 22, 2018

@laf laf merged commit 23740bc into librenms:master Apr 22, 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 the Device 🖥 label Apr 22, 2018

@laf

This comment has been minimized.

Member

laf commented Apr 22, 2018

image

@czilian czilian deleted the czilian:fhswitches branch Apr 24, 2018

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

device: Added support for FiberHome Switches S5800/S4800/S2800 (libre…
…nms#8569)

* Fiber Home Switch Device Support
- Basic OS support
- New Logo
- Sensor support
- Fiberhome MIBS (wriSMI)

* Fibrehome Switch Device Support
- add mempools discovery

* Fiberhome Switch Device Support
- remove mib bases high/low limit for FAN entity

* Fiberhome Switch Device Support
- remove unsupported processor high_limit option

* FiberHome Switch Device Support
- snmpprec files for S28/S48/S58 added
- removed not used FH MIB files

* Fiberhome Switch Device Support
- add snmprec for FHN5800 FHN4800 FHN2800

* Update fiberhome.svg

* FiberHome Switch Device Support - changes as per request

* FiberHome Switch Device Support
- adjusted code to PSR2 standard

* FiberHome Switch Device Support - add tests

* FiberHome Switch Device Support - added back fiberhome.png since currently expected in other json

* FiberHome Switches Device Support - Deleted fiberhome.svg (for now)

* Fiberhome Switch Device Support - corrected wrong MIB renaming

* Update fiberhome-switch.inc.php

* Update fiberhome-switch.inc.php

* Update fiberhome-switch.inc.php

* Update fiberhome-switch_fh4800.json

* Update fiberhome-switch.json

* Update fiberhome-switch_fh2800.json

* Update fiberhome-switch.yaml

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

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