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

Added 3 Phase APC UPS Support issue #2733 & #5504 #5558

Merged
merged 18 commits into from May 5, 2017

Conversation

Projects
None yet
@NerdBlender
Contributor

NerdBlender commented Jan 23, 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?
@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Jan 23, 2017

Thank you for submitting a PR @NerdBlender! We have found the following @laf, @tuxis-ie and @Rosiak based on the history of these files to review this PR.

mention-bot commented Jan 23, 2017

Thank you for submitting a PR @NerdBlender! We have found the following @laf, @tuxis-ie and @Rosiak based on the history of these files to review this PR.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

LibreNMS-CI commented Jan 23, 2017

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

@NerdBlender NerdBlender changed the title from Added 3 Phase APC UPS Support issue #2733 to Added 3 Phase APC UPS Support issue #2733 & #5504 Jan 23, 2017

@NerdBlender

This comment has been minimized.

Show comment
Hide comment
@NerdBlender

NerdBlender Jan 23, 2017

Contributor

This should also work for issue number #5504

Contributor

NerdBlender commented Jan 23, 2017

This should also work for issue number #5504

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Jan 24, 2017

Member

I actually redid all the APC code awhile ago here: https://github.com/murrant/librenms/tree/apc-cleanup
but didn't merge it because I renamed some rrd files in some places and never got back to that.

Here is the code I used with an snmpwalk to handle any number of phases, does that work for you?

    $oids = snmpwalk_cache_oid($device, 'upsHighPrecOutputCurrent', array(), 'PowerNet-MIB');
    if (empty($oids)) {
        $oids = snmpwalk_cache_oid($device, 'upsAdvOutputCurrent', $oids, 'PowerNet-MIB');
    }

    foreach ($oids as $index => $data) {
        $type = 'apcUPS';
        $descr = 'Output Current';

        if (isset($data['upsHighPrecOutputCurrent'])) {
            $current_oid = '.1.3.6.1.4.1.318.1.1.1.4.3.4.' . $index;
            $divisor = 10;
            $current = $data['upsHighPrecOutputCurrent'] / $divisor;
        } else {
            $current_oid = '.1.3.6.1.4.1.318.1.1.1.4.2.4.' . $index;
            $divisor = 1;
            $current = $data['upsAdvOutputCurrent'];
        }

        discover_sensor($valid['sensor'], 'current', $device, $current_oid, $index, $type, $descr, $divisor, 1, null, null, null, null, $current);
    }
Member

murrant commented Jan 24, 2017

I actually redid all the APC code awhile ago here: https://github.com/murrant/librenms/tree/apc-cleanup
but didn't merge it because I renamed some rrd files in some places and never got back to that.

Here is the code I used with an snmpwalk to handle any number of phases, does that work for you?

    $oids = snmpwalk_cache_oid($device, 'upsHighPrecOutputCurrent', array(), 'PowerNet-MIB');
    if (empty($oids)) {
        $oids = snmpwalk_cache_oid($device, 'upsAdvOutputCurrent', $oids, 'PowerNet-MIB');
    }

    foreach ($oids as $index => $data) {
        $type = 'apcUPS';
        $descr = 'Output Current';

        if (isset($data['upsHighPrecOutputCurrent'])) {
            $current_oid = '.1.3.6.1.4.1.318.1.1.1.4.3.4.' . $index;
            $divisor = 10;
            $current = $data['upsHighPrecOutputCurrent'] / $divisor;
        } else {
            $current_oid = '.1.3.6.1.4.1.318.1.1.1.4.2.4.' . $index;
            $divisor = 1;
            $current = $data['upsAdvOutputCurrent'];
        }

        discover_sensor($valid['sensor'], 'current', $device, $current_oid, $index, $type, $descr, $divisor, 1, null, null, null, null, $current);
    }
@laf

A lot of the snmp gets can be changed to multi gets (or walks but I don't know the output for these)

@NerdBlender

This comment has been minimized.

Show comment
Hide comment
@NerdBlender

NerdBlender Jan 25, 2017

Contributor

@murrant - that doesn't work - as upsHighPrecOutputCurrent is always present - so $oids will always be populated - but i think i can use that to make something that works.

Contributor

NerdBlender commented Jan 25, 2017

@murrant - that doesn't work - as upsHighPrecOutputCurrent is always present - so $oids will always be populated - but i think i can use that to make something that works.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

LibreNMS-CI commented Jan 26, 2017

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

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Jan 26, 2017

Member

I've asked the people in the issues if they can test this.

Member

laf commented Jan 26, 2017

I've asked the people in the issues if they can test this.

@wriedel

This comment has been minimized.

Show comment
Hide comment
@wriedel

wriedel Jan 26, 2017

Hi Neil,
many thanks for picking this up. Currently it's not really working for me. I have two apc-surt-20000 and one is now showing voltage for two phases while the other still for one while in reality I have three phases

here some debugs:
http://pastebin.com/0KDGyugw
http://pastebin.com/eYuwv5Vi
and SNMP walks
http://pastebin.com/PuT83sTP
http://pastebin.com/5tVnjQuR

I am not sure on dependencies on #5626 as this bug broke my discovery process.

Thank you,
Wolfgang

wriedel commented Jan 26, 2017

Hi Neil,
many thanks for picking this up. Currently it's not really working for me. I have two apc-surt-20000 and one is now showing voltage for two phases while the other still for one while in reality I have three phases

here some debugs:
http://pastebin.com/0KDGyugw
http://pastebin.com/eYuwv5Vi
and SNMP walks
http://pastebin.com/PuT83sTP
http://pastebin.com/5tVnjQuR

I am not sure on dependencies on #5626 as this bug broke my discovery process.

Thank you,
Wolfgang

@NerdBlender

This comment has been minimized.

Show comment
Hide comment
@NerdBlender

NerdBlender Jan 27, 2017

Contributor

@wriedel - Can you get me the output of /usr/bin/snmpbulkwalk -v2c -c 'YOUR COMMUNITY' -OQUs -m IF-MIB -M /opt/librenms/mibs:/opt/librenms/mibs/apc udp:apc-surt-20000-b.toocoolforyou.net:161 1.3.6.1.4.1.318

Your snmpwalk is missing the APC info

Contributor

NerdBlender commented Jan 27, 2017

@wriedel - Can you get me the output of /usr/bin/snmpbulkwalk -v2c -c 'YOUR COMMUNITY' -OQUs -m IF-MIB -M /opt/librenms/mibs:/opt/librenms/mibs/apc udp:apc-surt-20000-b.toocoolforyou.net:161 1.3.6.1.4.1.318

Your snmpwalk is missing the APC info

@NerdBlender

This comment has been minimized.

Show comment
Hide comment
@NerdBlender

NerdBlender Jan 27, 2017

Contributor

@wriedel - Try this latest version - i have changed to another OID to use for detection.

Contributor

NerdBlender commented Jan 27, 2017

@wriedel - Try this latest version - i have changed to another OID to use for detection.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

LibreNMS-CI commented Jan 27, 2017

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

@wriedel

This comment has been minimized.

Show comment
Hide comment
@wriedel

wriedel Jan 27, 2017

Hi, here the snmpwalk you asked about: http://pastebin.com/k8wgbnMC

wriedel commented on f85c68e Jan 27, 2017

Hi, here the snmpwalk you asked about: http://pastebin.com/k8wgbnMC

@NerdBlender

This comment has been minimized.

Show comment
Hide comment
@NerdBlender

NerdBlender Jan 27, 2017

Contributor

Sorry, that should have been:

/usr/bin/snmpbulkwalk -v2c -c 'YOUR COMMUNITY' -OQUs -m PowerNet-MIB -M /opt/librenms/mibs:/opt/librenms/mibs/apc udp:apc-surt-20000-b.toocoolforyou.net:161 1.3.6.1.4.1.318

However the latest update I did this morning make the code read the number of phases correctly.

Contributor

NerdBlender commented Jan 27, 2017

Sorry, that should have been:

/usr/bin/snmpbulkwalk -v2c -c 'YOUR COMMUNITY' -OQUs -m PowerNet-MIB -M /opt/librenms/mibs:/opt/librenms/mibs/apc udp:apc-surt-20000-b.toocoolforyou.net:161 1.3.6.1.4.1.318

However the latest update I did this morning make the code read the number of phases correctly.

@wriedel

This comment has been minimized.

Show comment
Hide comment
@wriedel

wriedel Jan 27, 2017

OK, here we are: http://pastebin.com/KcLPhCSY
To actually test it I need to do this, right?

gitpull=5558
cd /opt/librenms/
wget https://patch-diff.githubusercontent.com/raw/librenms/librenms/pull/$gitpull.diff
git apply $gitpull.diff

wriedel commented Jan 27, 2017

OK, here we are: http://pastebin.com/KcLPhCSY
To actually test it I need to do this, right?

gitpull=5558
cd /opt/librenms/
wget https://patch-diff.githubusercontent.com/raw/librenms/librenms/pull/$gitpull.diff
git apply $gitpull.diff
@NerdBlender

This comment has been minimized.

Show comment
Hide comment
@NerdBlender

NerdBlender Jan 27, 2017

Contributor

I'm not sure about the diff pull - i'm not a git expert @laf might suggest the best method.

However, I think I know why this is wrong. This UPS can be configured with 3phase in and 3phase out, OR with 3phase in and a single phase output which looks to be how yours is configured, so i will need to add some extra logic to check for the number of output phases.

Contributor

NerdBlender commented Jan 27, 2017

I'm not sure about the diff pull - i'm not a git expert @laf might suggest the best method.

However, I think I know why this is wrong. This UPS can be configured with 3phase in and 3phase out, OR with 3phase in and a single phase output which looks to be how yours is configured, so i will need to add some extra logic to check for the number of output phases.

@wriedel

This comment has been minimized.

Show comment
Hide comment
@wriedel

wriedel Jan 27, 2017

Yes, that is correct.
In fact you can hard-wire 1 or 3 Phase in and 1 or 3 Phase out.
Currently I have 3 Phase in and 1 Phase out as there is a dependency on 1 Phase per battery pack and we just have one battery pack per UPS.
On Cacti I just pulled all 3 Phases in and out all the time as if unused they're just zero.

wriedel commented Jan 27, 2017

Yes, that is correct.
In fact you can hard-wire 1 or 3 Phase in and 1 or 3 Phase out.
Currently I have 3 Phase in and 1 Phase out as there is a dependency on 1 Phase per battery pack and we just have one battery pack per UPS.
On Cacti I just pulled all 3 Phases in and out all the time as if unused they're just zero.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

LibreNMS-CI commented Jan 27, 2017

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

@NerdBlender

This comment has been minimized.

Show comment
Hide comment
@NerdBlender

NerdBlender Jan 28, 2017

Contributor

@wriedel - this should work for you now, i managed to test with someone with a similar setup, although a different model.

Contributor

NerdBlender commented Jan 28, 2017

@wriedel - this should work for you now, i managed to test with someone with a similar setup, although a different model.

@wriedel

This comment has been minimized.

Show comment
Hide comment
@wriedel

wriedel Jan 28, 2017

@NerdBlender good Morning,
is this change already in the current diff for 5558?
If so, sorry still no luck but not sure if I need to delete and add the device again?

After applying the patch I see this
2017-01-28 11:09:52 Sensor Deleted: voltage apc 3.2.1.0 Input
2017-01-28 11:09:52 Sensor Deleted: voltage apc 4.2.1.0 Output
2017-01-28 11:09:51 Sensor Added: voltage apcIn 0 Input
2017-01-28 11:09:51 Sensor Added: voltage apcOut 0 Output
but still just one graph.

Attached the current debugs for discovery and poller.
Wolfgang

apc-surt-20000-a.toocoolforyou.net.log.txt

wriedel commented Jan 28, 2017

@NerdBlender good Morning,
is this change already in the current diff for 5558?
If so, sorry still no luck but not sure if I need to delete and add the device again?

After applying the patch I see this
2017-01-28 11:09:52 Sensor Deleted: voltage apc 3.2.1.0 Input
2017-01-28 11:09:52 Sensor Deleted: voltage apc 4.2.1.0 Output
2017-01-28 11:09:51 Sensor Added: voltage apcIn 0 Input
2017-01-28 11:09:51 Sensor Added: voltage apcOut 0 Output
but still just one graph.

Attached the current debugs for discovery and poller.
Wolfgang

apc-surt-20000-a.toocoolforyou.net.log.txt

@NerdBlender

This comment has been minimized.

Show comment
Hide comment
@NerdBlender

NerdBlender Jan 30, 2017

Contributor

@wriedel could you get me the output of /usr/bin/snmpbulkwalk -v2c -c 'YOUR COMMUNITY' -OQUs -m PowerNet-MIB -M /opt/librenms/mibs:/opt/librenms/mibs/apc udp:apc-surt-20000-b.toocoolforyou.net:161 1.3.6.1.4.1.318

Pastebin removed the last one.

Contributor

NerdBlender commented Jan 30, 2017

@wriedel could you get me the output of /usr/bin/snmpbulkwalk -v2c -c 'YOUR COMMUNITY' -OQUs -m PowerNet-MIB -M /opt/librenms/mibs:/opt/librenms/mibs/apc udp:apc-surt-20000-b.toocoolforyou.net:161 1.3.6.1.4.1.318

Pastebin removed the last one.

@wriedel

This comment has been minimized.

Show comment
Hide comment
@wriedel

wriedel Jan 30, 2017

@NerdBlender
here we go: http://pastebin.com/JANzu3af

Many thanks!!!
Wolfgang

wriedel commented Jan 30, 2017

@NerdBlender
here we go: http://pastebin.com/JANzu3af

Many thanks!!!
Wolfgang

@kakopedreros

This comment has been minimized.

Show comment
Hide comment
@kakopedreros

kakopedreros Mar 31, 2017

Contributor

@NerdBlender maybe is what you say. The management card doesn't recognize it is a 3-phase UPS...

A pic of the INPUT connectors of the UPS...

img_0816

Contributor

kakopedreros commented Mar 31, 2017

@NerdBlender maybe is what you say. The management card doesn't recognize it is a 3-phase UPS...

A pic of the INPUT connectors of the UPS...

img_0816

@NerdBlender

This comment has been minimized.

Show comment
Hide comment
@NerdBlender

NerdBlender Apr 3, 2017

Contributor

@laf @murrant any ideas if there is anything else that can be done to work out where the issue is?

The SNMP walk seem to be missing the OIDs that handle 3 phase UPS data output. Ever seen anything like that before?

Contributor

NerdBlender commented Apr 3, 2017

@laf @murrant any ideas if there is anything else that can be done to work out where the issue is?

The SNMP walk seem to be missing the OIDs that handle 3 phase UPS data output. Ever seen anything like that before?

@wriedel

This comment has been minimized.

Show comment
Hide comment
@wriedel

wriedel Apr 4, 2017

@NerdBlender if my memory serve me right, it's implemented differently on these older UPS which didn't have the 3 Phase measurement capabilities and therefore the OID seem to be different:

Input Current: .1.3.6.1.4.1.318.1.1.1.4.2.4.0
Input Power: .1.3.6.1.4.1.318.1.1.1.4.2.3.0
Input Voltage: .1.3.6.1.4.1.318.1.1.1.3.2.1.0
Output Current : .1.3.6.1.4.1.318.1.1.1.4.2.4.0
Output Voltage: .1.3.6.1.4.1.318.1.1.1.4.2.1.0

I guess you need to drop the 3Phase visualization and treat it as a single Phase UPS.
Everything else should be similar I guess but these are End Of Sale and End Of Live anyway, right ;-)

Wolfgang

wriedel commented Apr 4, 2017

@NerdBlender if my memory serve me right, it's implemented differently on these older UPS which didn't have the 3 Phase measurement capabilities and therefore the OID seem to be different:

Input Current: .1.3.6.1.4.1.318.1.1.1.4.2.4.0
Input Power: .1.3.6.1.4.1.318.1.1.1.4.2.3.0
Input Voltage: .1.3.6.1.4.1.318.1.1.1.3.2.1.0
Output Current : .1.3.6.1.4.1.318.1.1.1.4.2.4.0
Output Voltage: .1.3.6.1.4.1.318.1.1.1.4.2.1.0

I guess you need to drop the 3Phase visualization and treat it as a single Phase UPS.
Everything else should be similar I guess but these are End Of Sale and End Of Live anyway, right ;-)

Wolfgang

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Apr 5, 2017

Member

@wriedel I guess if there are oid's available for the older models then you can use those, if we record models / versions in polling you could also base the detection on that although imho that's messy.

Member

laf commented Apr 5, 2017

@wriedel I guess if there are oid's available for the older models then you can use those, if we record models / versions in polling you could also base the detection on that although imho that's messy.

@wriedel

This comment has been minimized.

Show comment
Hide comment
@wriedel

wriedel Apr 5, 2017

@laf @NerdBlender @kakopedreros @murrant my point is that SURT 8000 / SURT 10000 / and so on may have 3Phase connectivity but even inside APC didn't really count as 3 Phase UPS.
Different product line and support team!
So it would make sense to put them into the non 3Phase bucket and use the other OID's.

wriedel commented Apr 5, 2017

@laf @NerdBlender @kakopedreros @murrant my point is that SURT 8000 / SURT 10000 / and so on may have 3Phase connectivity but even inside APC didn't really count as 3 Phase UPS.
Different product line and support team!
So it would make sense to put them into the non 3Phase bucket and use the other OID's.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Apr 5, 2017

Member

I'll trust what you say :)

Our goal is not to lose data so if what you propose doesn't do that and is the correct choice then go ahead.

Member

laf commented Apr 5, 2017

I'll trust what you say :)

Our goal is not to lose data so if what you propose doesn't do that and is the correct choice then go ahead.

@f0o

f0o approved these changes Apr 6, 2017

@f0o

This comment has been minimized.

Show comment
Hide comment
@f0o

f0o Apr 6, 2017

Member

@laf is your changes request still valid? I couldnt find your points in the recent files

Member

f0o commented Apr 6, 2017

@laf is your changes request still valid? I couldnt find your points in the recent files

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Apr 6, 2017

Member

Seems ok, if @NerdBlender can just confirm from my last comments we should be good to go.

Member

laf commented Apr 6, 2017

Seems ok, if @NerdBlender can just confirm from my last comments we should be good to go.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf
Member

laf commented Apr 12, 2017

@NerdBlender

This comment has been minimized.

Show comment
Hide comment
@NerdBlender

NerdBlender Apr 18, 2017

Contributor

@laf - I am good with your comments.

Let me know if you need anything from me.

Contributor

NerdBlender commented Apr 18, 2017

@laf - I am good with your comments.

Let me know if you need anything from me.

@wriedel

This comment has been minimized.

Show comment
Hide comment
@wriedel

wriedel Apr 21, 2017

Hi Folks, just wonder what's missing so we can finally close this PR and get it out of the door ;-)

wriedel commented Apr 21, 2017

Hi Folks, just wonder what's missing so we can finally close this PR and get it out of the door ;-)

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Apr 22, 2017

Member

I guess just the conflict to fix + sign the new CLA

Member

laf commented Apr 22, 2017

I guess just the conflict to fix + sign the new CLA

@wriedel

This comment has been minimized.

Show comment
Hide comment
@wriedel

wriedel Apr 23, 2017

@NerdBlender

yes, too bad as this now breaks the current test setup and deleted all my graphs 👎

2017-04-21 18:30:21 Sensor Deleted: voltage apcUPS 3.1.3.1.1.2 Phase 2 Input
2017-04-21 18:30:21 Sensor Deleted: voltage apcUPS 3.1.3.1.1.3 Phase 3 Input
2017-04-21 18:30:21 Sensor Deleted: voltage apcUPS 3.1.3.2.1.1 Phase 1 Bypass Input
2017-04-21 18:30:21 Sensor Deleted: voltage apc 2.2.8.0 Battery Bus
2017-04-21 18:30:21 Sensor Deleted: voltage apcUPS 1.1.1 Phase 1 Output
2017-04-21 18:30:21 Sensor Deleted: voltage apcUPS 3.1.3.1.1.1 Phase 1 Input

git pull NerdBlender issue-2733
From https://github.com/NerdBlender/librenms

  • branch issue-2733 -> FETCH_HEAD
    Auto-merging includes/discovery/sensors/pre-cache/apc.inc.php
    CONFLICT (content): Merge conflict in includes/discovery/sensors/pre-cache/apc.inc.php
    Automatic merge failed; fix conflicts and then commit the result.

./scripts/github-apply 5558
also created tons of errors..

wriedel commented Apr 23, 2017

@NerdBlender

yes, too bad as this now breaks the current test setup and deleted all my graphs 👎

2017-04-21 18:30:21 Sensor Deleted: voltage apcUPS 3.1.3.1.1.2 Phase 2 Input
2017-04-21 18:30:21 Sensor Deleted: voltage apcUPS 3.1.3.1.1.3 Phase 3 Input
2017-04-21 18:30:21 Sensor Deleted: voltage apcUPS 3.1.3.2.1.1 Phase 1 Bypass Input
2017-04-21 18:30:21 Sensor Deleted: voltage apc 2.2.8.0 Battery Bus
2017-04-21 18:30:21 Sensor Deleted: voltage apcUPS 1.1.1 Phase 1 Output
2017-04-21 18:30:21 Sensor Deleted: voltage apcUPS 3.1.3.1.1.1 Phase 1 Input

git pull NerdBlender issue-2733
From https://github.com/NerdBlender/librenms

  • branch issue-2733 -> FETCH_HEAD
    Auto-merging includes/discovery/sensors/pre-cache/apc.inc.php
    CONFLICT (content): Merge conflict in includes/discovery/sensors/pre-cache/apc.inc.php
    Automatic merge failed; fix conflicts and then commit the result.

./scripts/github-apply 5558
also created tons of errors..

@CLAassistant

This comment has been minimized.

Show comment
Hide comment
@CLAassistant

CLAassistant May 1, 2017

CLA assistant check
All committers have signed the CLA.

CLAassistant commented May 1, 2017

CLA assistant check
All committers have signed the CLA.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf May 1, 2017

Member

@NerdBlender I've rebased this now, if you can just sign the new CLA: https://cla-assistant.io/librenms/librenms?pullRequest=5558 then we can get it merged.

Member

laf commented May 1, 2017

@NerdBlender I've rebased this now, if you can just sign the new CLA: https://cla-assistant.io/librenms/librenms?pullRequest=5558 then we can get it merged.

@NerdBlender

This comment has been minimized.

Show comment
Hide comment
@NerdBlender

NerdBlender May 2, 2017

Contributor

Hi All - Apologies, I have been travelling and haven't had opportunity to catch up with this. Thanks @laf for re-basing, I have signed the agreement

Contributor

NerdBlender commented May 2, 2017

Hi All - Apologies, I have been travelling and haven't had opportunity to catch up with this. Thanks @laf for re-basing, I have signed the agreement

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

LibreNMS-CI commented May 2, 2017

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

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf May 2, 2017

Member

I've pushed some small code formatting but also the type from apcUPS to apc to keep things consistent.

@NerdBlender can you double check this is ok.

Member

laf commented May 2, 2017

I've pushed some small code formatting but also the type from apcUPS to apc to keep things consistent.

@NerdBlender can you double check this is ok.

@NerdBlender

This comment has been minimized.

Show comment
Hide comment
@NerdBlender

NerdBlender May 3, 2017

Contributor

@laf all looks good to me.

Contributor

NerdBlender commented May 3, 2017

@laf all looks good to me.

@laf

laf approved these changes May 3, 2017

@librenms/reviewers

Tagging for merge in 48 hours.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

LibreNMS-CI commented May 5, 2017

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

@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier May 5, 2017

The inspection completed: 667 Issues, 21 Patches

scrutinizer-notifier commented May 5, 2017

The inspection completed: 667 Issues, 21 Patches

@laf laf merged commit a368a73 into librenms:master May 5, 2017

3 checks passed

Auto-Deploy Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
@lock

This comment has been minimized.

Show comment
Hide comment
@lock

lock bot May 18, 2018

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

lock bot commented May 18, 2018

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

@lock lock bot locked as resolved and limited conversation to collaborators May 18, 2018

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