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 - ICT Digital Power Supply #6369

Merged
merged 6 commits into from Apr 12, 2017

Conversation

Projects
None yet
5 participants
@EnzoZafra
Contributor

EnzoZafra commented Apr 5, 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

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
Show outdated Hide outdated includes/discovery/sensors/current/ict-psu.inc.php
$outputCurrent = trim(snmp_get($device, $outputCurrent_oid, '-Oqv'), '" ');
if (!empty($outputCurrent)) {
$divisor = 1;
$index = '11.8.0';

This comment has been minimized.

@laf

laf Apr 6, 2017

Member

$index can just be 0 in this instance.

@laf

laf Apr 6, 2017

Member

$index can just be 0 in this instance.

Show outdated Hide outdated includes/discovery/sensors/voltage/ict-psu.inc.php
$inputVoltage = trim(snmp_get($device, $inputVoltage_oid, '-Oqv'), '" ');
if (!empty($inputVoltage)) {
$divisor = 1;
$index = '11.6.0';

This comment has been minimized.

@laf

laf Apr 6, 2017

Member

$index You can use 0 and 1 here as you know how many you have, the alternative is to use the OID name if one is available from the mib like InputVoltage

@laf

laf Apr 6, 2017

Member

$index You can use 0 and 1 here as you know how many you have, the alternative is to use the OID name if one is available from the mib like InputVoltage

Show outdated Hide outdated includes/discovery/sensors/voltage/ict-psu.inc.php
$outputVoltage = trim(snmp_get($device, $outputVoltage_oid, '-Oqv'), '" ');
if (!empty($outputVoltage)) {
$divisor = 1;
$index = '11.7.0';

This comment has been minimized.

@laf

laf Apr 6, 2017

Member

Same as last comment.

@laf

laf Apr 6, 2017

Member

Same as last comment.

Show outdated Hide outdated includes/discovery/sensors/current/ict-psu.inc.php
// Output Current
// SNMPv2-SMI::enterprises.39145.11.8.0 = STRING: "0.4" -- outputCurrent
$outputCurrent_oid = '.1.3.6.1.4.1.39145.11.8.0';

This comment has been minimized.

@laf

laf Apr 6, 2017

Member

You've used the numeric OID all over but have provided a mib, it's much cleaner to use the name from the MIB if you can.

@laf

laf Apr 6, 2017

Member

You've used the numeric OID all over but have provided a mib, it's much cleaner to use the name from the MIB if you can.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@EnzoZafra

This comment has been minimized.

Show comment
Hide comment
@EnzoZafra

EnzoZafra Apr 11, 2017

Contributor

Hey @laf, I fixed the code upon your requests but it still says 'Changes requested'. Not sure if I did anything wrong

Contributor

EnzoZafra commented Apr 11, 2017

Hey @laf, I fixed the code upon your requests but it still says 'Changes requested'. Not sure if I did anything wrong

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Apr 12, 2017

Member

@EnzoZafra laf needs to manually change his review to make that go away ;)

Member

murrant commented Apr 12, 2017

@EnzoZafra laf needs to manually change his review to make that go away ;)

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier Apr 12, 2017

The inspection completed: 1 new issues, 3 updated code elements

The inspection completed: 1 new issues, 3 updated code elements

@murrant murrant merged commit bd3fd64 into librenms:master Apr 12, 2017

2 checks passed

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

@EnzoZafra EnzoZafra referenced this pull request Apr 13, 2017

Closed

New Device Support - ICT Digital Series Power Supply #6350

5 of 5 tasks complete

@EnzoZafra EnzoZafra deleted the EnzoZafra:issue-6350 branch Apr 13, 2017

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

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