fix: Change rfc1628 'state' (est. runtime and on battery) to runtime #6158

Merged
merged 4 commits into from Mar 12, 2017

Conversation

Projects
None yet
6 participants
@murrant
Member

murrant commented Mar 10, 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

@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Mar 10, 2017

Thank you for submitting a PR @murrant! We have found the following @laf and @einhirn based on the history of these files to review this PR.

Thank you for submitting a PR @murrant! We have found the following @laf and @einhirn based on the history of these files to review this PR.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@FTBZ

This comment has been minimized.

Show comment
Hide comment
@FTBZ

FTBZ Mar 10, 2017

Contributor

Thanks for all your job @murrant. Not sure that these graphs are working enough in "runtime" type. Because when you're not on battery it returning nothing.

screen shot 2017-03-10 at 07 27 06

The "seconds on battery" graph a value of 0, but the "minutes remaining" graph isn't working (Error Drawing Graph).

Contributor

FTBZ commented Mar 10, 2017

Thanks for all your job @murrant. Not sure that these graphs are working enough in "runtime" type. Because when you're not on battery it returning nothing.

screen shot 2017-03-10 at 07 27 06

The "seconds on battery" graph a value of 0, but the "minutes remaining" graph isn't working (Error Drawing Graph).

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Mar 10, 2017

Member

Yeah, not sure what the issue is with the graphs. I'l check it out when I get a chance.

Member

murrant commented Mar 10, 2017

Yeah, not sure what the issue is with the graphs. I'l check it out when I get a chance.

Fix up the graph error ( : in ds name).
Change seconds on battery to minutes so the value is in minutes
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment

@FTBZ FTBZ referenced this pull request Mar 12, 2017

Merged

fix: Adding state sensor support for RFC1628 UPS #6153

2 of 2 tasks complete
@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Mar 12, 2017

Member

I thought I'd commented on this last night, I updated #6153 before seeing this.

I can remove that change no problem but the runtime code should just be an snmp_get shouldn't it as it's only one OID.

Member

laf commented Mar 12, 2017

I thought I'd commented on this last night, I updated #6153 before seeing this.

I can remove that change no problem but the runtime code should just be an snmp_get shouldn't it as it's only one OID.

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Mar 12, 2017

Member

You commented on the application discovery PR, I was confused at first :-). Let me check the MIB, unless you already did...

Member

murrant commented Mar 12, 2017

You commented on the application discovery PR, I was confused at first :-). Let me check the MIB, unless you already did...

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Mar 12, 2017

Member

Was pushing my lack of sleep last night :/

image

Member

laf commented Mar 12, 2017

Was pushing my lack of sleep last night :/

image

@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 Mar 12, 2017

The inspection completed: No new issues

The inspection completed: No new issues

@laf laf merged commit 4ae4847 into librenms:master Mar 12, 2017

2 checks passed

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

@murrant murrant deleted the murrant:rfc-runtime branch Mar 13, 2017

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