newdevice: Added support for Cyberpower PDU #6013

Merged
merged 72 commits into from Mar 3, 2017

Conversation

Projects
None yet
6 participants
@VVelox
Contributor

VVelox commented Feb 26, 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.

  • 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

Add Cyberpower PDU support, including voltage and current polling.

VVelox and others added some commits Feb 15, 2017

kitsune
*rename it so it can be called via the apps page
*setup the apps page to call it
tests/snmpsim/cyberpower.snmprec
@@ -0,0 +1,2 @@
+1.3.6.1.2.1.1.1.0|4|Power Manager
+1.3.6.1.2.1.1.2.0|6|.1.3.6.1.4.1.3808.1.1.3

This comment has been minimized.

@laf

laf Mar 1, 2017

Member

To keep things the same, drop the leading . from .1.3.6...: 1.3.6.1.4.1.3808.1.1.3

@laf

laf Mar 1, 2017

Member

To keep things the same, drop the leading . from .1.3.6...: 1.3.6.1.4.1.3808.1.1.3

@laf

Couple of changes + Can you provide a logo (needs the yaml file updating when you do)

@laf laf changed the title from add Cyberpower PDU support to newdevice: Added support for Cyberpower PDU Mar 1, 2017

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
+<?php
+
+// Cyberpower Voltages
+if ($device['os'] == 'cyberpower') {

This comment has been minimized.

@laf

laf Mar 2, 2017

Member

Still don't need this :)

@laf

laf Mar 2, 2017

Member

Still don't need this :)

This comment has been minimized.

@VVelox

VVelox Mar 3, 2017

Contributor

Removed. :)

@VVelox

VVelox Mar 3, 2017

Contributor

Removed. :)

includes/polling/os/cyberpower.inc.php
@@ -0,0 +1,12 @@
+<?php
+
+$OIDs=array(

This comment has been minimized.

@laf

laf Mar 2, 2017

Member

Can you lower case all of your variables please, use _ to separate where you need to

@laf

laf Mar 2, 2017

Member

Can you lower case all of your variables please, use _ to separate where you need to

This comment has been minimized.

@VVelox

VVelox Mar 3, 2017

Contributor

Done. :)

@VVelox

VVelox Mar 3, 2017

Contributor

Done. :)

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment

VVelox added some commits Mar 3, 2017

LC
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@VVelox

This comment has been minimized.

Show comment
Hide comment
@VVelox

VVelox Mar 3, 2017

Contributor

Done, except for reverting the logo change.

https://www.cyberpowersystems.com/ is the company in question. http://www.cyberpowerpc.com/ is where the current logo is from and is a different company.

Pulled their logo from their favicon and converted it to a SVG previously.

Contributor

VVelox commented Mar 3, 2017

Done, except for reverting the logo change.

https://www.cyberpowersystems.com/ is the company in question. http://www.cyberpowerpc.com/ is where the current logo is from and is a different company.

Pulled their logo from their favicon and converted it to a SVG previously.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@VVelox

This comment has been minimized.

Show comment
Hide comment
@VVelox

VVelox Mar 3, 2017

Contributor

Updated with the logo from their website and replaced the OS with their favicon.

Contributor

VVelox commented Mar 3, 2017

Updated with the logo from their website and replaced the OS with their favicon.

disable port polling
This device has a buggy SNMP implementation and port polling does not work.
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
+discovery:
+ - sysObjectId: .1.3.6.1.4.1.3808.1.1.3
+poller_modules:
+ ports: 0

This comment has been minimized.

@laf

laf Mar 3, 2017

Member

Do they not provide any port info at all?

@laf

laf Mar 3, 2017

Member

Do they not provide any port info at all?

This comment has been minimized.

@VVelox

VVelox Mar 3, 2017

Contributor

Nothing usable. SNMP for it is effectively broken when it comes to that. It provides one interface and the stats never change.

@VVelox

VVelox Mar 3, 2017

Contributor

Nothing usable. SNMP for it is effectively broken when it comes to that. It provides one interface and the stats never change.

This comment has been minimized.

@dorkmatt

dorkmatt Jun 14, 2017

Contributor

Out of curiosity, which firmware version was used here? It looks like Cyberpower is updating the firmware regularly, wonder if they know about the lack of SNMP port counters bug?

@dorkmatt

dorkmatt Jun 14, 2017

Contributor

Out of curiosity, which firmware version was used here? It looks like Cyberpower is updating the firmware regularly, wonder if they know about the lack of SNMP port counters bug?

@laf

After the question about ports this is good to go.

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Mar 3, 2017

Member

@VVelox FYI, the OSDiscovery tests are alphabetical, please keep them that way ;)

Member

murrant commented Mar 3, 2017

@VVelox FYI, the OSDiscovery tests are alphabetical, please keep them that way ;)

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Mar 3, 2017

Member

Didn't even spot that.

Member

laf commented Mar 3, 2017

Didn't even spot that.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Mar 3, 2017

Member

Yeah, I didn't notice until there was a merge conflict.

Member

murrant commented Mar 3, 2017

Yeah, I didn't notice until there was a merge conflict.

@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier Mar 3, 2017

The inspection completed: 1 updated code elements

The inspection completed: 1 updated code elements

@laf laf merged commit 5805a55 into librenms:master Mar 3, 2017

2 checks passed

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

This comment has been minimized.

Show comment
Hide comment
@lock

lock bot May 17, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

lock bot commented May 17, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

@librenms librenms locked as resolved and limited conversation to collaborators May 17, 2018

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