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

device: MGE UPS support improvement (incl. traps) #9301

Merged
merged 13 commits into from Oct 17, 2018

Conversation

Projects
None yet
4 participants
@PipoCanaja
Contributor

PipoCanaja commented Oct 5, 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
After you are done testing, you can remove the changes with ./scripts/github-remove. If there are schema changes, you can ask on discord how to revert.

@PipoCanaja PipoCanaja changed the title from Mge ups support galaxy to device: MGE UPS support improvement Oct 5, 2018

Show resolved Hide resolved includes/discovery/sensors/load/rfc1628.inc.php Outdated

@PipoCanaja PipoCanaja changed the title from device: MGE UPS support improvement to WIP - device: MGE UPS support improvement Oct 6, 2018

@PipoCanaja PipoCanaja changed the title from WIP - device: MGE UPS support improvement to device: MGE UPS support improvement Oct 7, 2018

@PipoCanaja PipoCanaja changed the title from device: MGE UPS support improvement to device: MGE UPS support improvement (incl. traps) Oct 9, 2018

Show resolved Hide resolved includes/discovery/sensors/voltage/rfc1628.inc.php Outdated
power:
data:
-
oid: upsmgOutput

This comment has been minimized.

@laf

laf Oct 9, 2018

Member

For all of these, you've used a high level folder / OID when you should be using the Table. See this pic:

image

So use upsmgOutputPhaseTable in this one example but the rest also need to be updated.

This comment has been minimized.

@PipoCanaja

PipoCanaja Oct 9, 2018

Contributor

Hello,

I changed it for all the Phase entries, but the "BadStatus" for instance is in the msgInput table and if I change it then it will convert a snmpwalk into multiple snmpget. Is it really better in that case ?

This comment has been minimized.

@laf

laf Oct 9, 2018

Member

It will always do a walk but if you will be re-using data within the folder OID then it will be fine to use it as we cache the original walk so long as you aren't walking a lot of data.

It's a trade off between walking too much data to cache and not caching enough and having to perform multiple walks. Neither is perfect just use the best option.

PipoCanaja added some commits Oct 9, 2018

@laf laf added the Device 🖥 label Oct 9, 2018

@laf

laf approved these changes Oct 9, 2018

LGTM now.

@laf laf added this to the 1.45 milestone Oct 9, 2018

@laf laf merged commit 2125c86 into librenms:master Oct 17, 2018

2 of 3 checks passed

codeclimate 4 issues to fix
Details
WIP ready for review
Details
license/cla Contributor License Agreement is signed.
Details
@nonobzh29

This comment has been minimized.

nonobzh29 commented Oct 18, 2018

This commit introduced a bug regarding the input and output voltage values.
It drops by a factor of 10 ! (23.2V instead of 232V).
Please fix it asap, I didn't like to be waked up by the alert last night after the update ...

@PipoCanaja

This comment has been minimized.

Contributor

PipoCanaja commented Oct 19, 2018

Hi @nonobzh29
It seems that all eaton devices are not behaving the same.
Could you PasteBin your device discovery.php and poller.php details ?
Thanx

@nonobzh29

This comment has been minimized.

nonobzh29 commented Oct 20, 2018

Hi @PipoCanaja
Discovery pastebin : https://pastebin.com/Kg1N30pp
Poller pastebin : https://pastebin.com/Ld46v4Dy

PipoCanaja added a commit to PipoCanaja/librenms that referenced this pull request Oct 20, 2018

laf added a commit that referenced this pull request Oct 26, 2018

fix: #9301: Eaton/APC Galaxy seems to be the exception with divisor 1…
…0 for voltage (#9356)

* Adjustment in #9301: Eaton Galaxy HW seems to be the exception here with divisor 10 for voltage

* fix for APC Galaxy UPS (and not Eaton anymore)

* remove Galaxy test data from eaton-mgeups files

* tests: APC Galaxy 7000 test data
@nonobzh29

This comment has been minimized.

nonobzh29 commented Oct 27, 2018

Hi @PipoCanaja
I confirm it's fixed now! Thanks!

@PipoCanaja PipoCanaja deleted the PipoCanaja:MGE_UPS_Support_Galaxy branch Oct 27, 2018

@murrant

This comment has been minimized.

Member

murrant commented Oct 28, 2018

This pull request has been mentioned on LibreNMS Community. There might be relevant details there:

https://community.librenms.org/t/v1-45-release-changelog-october-2018/6016/1

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