Skip to content
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

Icotera MIBs for 6400 and 6800 series. #9755

Open
wants to merge 9 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@funzoneq
Copy link

funzoneq commented Jan 29, 2019

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.

@CLAassistant

This comment has been minimized.

Copy link

CLAassistant commented Jan 29, 2019

CLA assistant check
All committers have signed the CLA.

@PipoCanaja

This comment has been minimized.

Copy link
Contributor

PipoCanaja commented Jan 29, 2019

Hello @funzoneq ,
Just to clarify, adding MIB files only does not change the behaviour of LibreNMS in any way. You'll need to add YAML file to describe the OID you are interested in, or develop php discovery code (if the OIDs are not simple enough to be handled via YAML)

https://docs.librenms.org/Developing/Support-New-OS/

PipoCanaja

@funzoneq

This comment has been minimized.

Copy link
Author

funzoneq commented Jan 29, 2019

@PipoCanaja Thanks for the tip. I've added a simple definitions file.

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Jan 29, 2019

@funzoneq we usually like an icon SVG (preferred) or PNG. And we need at least a simple test file. You can generate this with ./scripts/collect-snmp-data.php -h HOSTNAME

@funzoneq

This comment has been minimized.

Copy link
Author

funzoneq commented Jan 29, 2019

@murrant Thanks! I've added both a PNG and a snmprec for the Icotera.

@PipoCanaja

This comment has been minimized.

Copy link
Contributor

PipoCanaja commented Jan 30, 2019

@funzoneq Great.
Last step is to add a json file that contains the device data as it is recognized now. The goal of it is to do regression tests later on. This file will be built out of the snmprec.

You should use : ./scripts/save-test-data.php -o icotera

Each time you change the yaml file, you will have to re-run save-test-data.php for sure (and sometimes the collect-snmp-data.php as well, depending on the type of change you did).

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Jan 31, 2019

What version of snmpsim are you using? It didn't hang, it just didn't fork...

@funzoneq

This comment has been minimized.

Copy link
Author

funzoneq commented Jan 31, 2019

What version of snmpsim are you using? It didn't hang, it just didn't fork...

As included at the end of the previous comment:

snmpsim==0.4.6

@dorkmatt

This comment has been minimized.

Copy link
Contributor

dorkmatt commented Feb 4, 2019

Here's a tested includes/polling/os/icotera.php

<?php

// SNMPv2-MIB::sysDescr.0 = STRING: ICOTERA i6405-51 2.5.0
preg_match('/ICOTERA (.*) (.*)/', $device['sysDescr'], $matches); 
$version  = $matches[2];
$hardware = $matches[1];
$serial = trim(snmp_get($device, '.1.3.6.1.2.1.47.1.1.1.1.11.1', '-OQv', '', ''), '"');

funzoneq added some commits Feb 8, 2019

oid: transceiverDdmTxBiasCurrent
num_oid: '.1.3.6.1.4.1.29865.11.3.1.3.5.{{ $index }}'
index: 'transceiverDdmTxBiasCurrent.{{ $index }}'
user_func: format_float

This comment has been minimized.

Copy link
@murrant

murrant Feb 11, 2019

Member

Why do you want to limit the resolution of the data stored?

This comment has been minimized.

Copy link
@dorkmatt

dorkmatt Feb 11, 2019

Contributor

To match the device's CLI output resolution.

This comment has been minimized.

Copy link
@murrant

murrant Mar 5, 2019

Member

Please store the full precision and if you like, limit it in the webui.

-
oid: transceiverDdmVoltage
num_oid: '.1.3.6.1.4.1.29865.11.3.1.3.4.{{ $index }}'
index: 'transceiverDdmVoltage.{{ $index }}'

This comment has been minimized.

Copy link
@murrant

murrant Feb 11, 2019

Member

You don't need to specify index unless you have conflicting indexes with other sensors. It makes a sane index by default.

This comment has been minimized.

Copy link
@dorkmatt

dorkmatt Feb 11, 2019

Contributor

No table exists in the MIB.

This comment has been minimized.

Copy link
@murrant

murrant Apr 12, 2019

Member

That is irrelevant to my comment, please remove the index line.

@laf

This comment has been minimized.

Copy link
Member

laf commented Apr 9, 2019

@funzoneq Any update on this? It's so close that with a few tweaks as per Tony's comments we could get this merged.

@funzoneq

This comment has been minimized.

Copy link
Author

funzoneq commented Apr 12, 2019

@laf @murrant does this address your comments?

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Apr 12, 2019

Isn't your data wrong now that you have removed the divisors?

@funzoneq

This comment has been minimized.

Copy link
Author

funzoneq commented Apr 13, 2019

Yeah it is, but you commented to "store the full precision and if you like, limit it in the webui". So I thought that included the divisors as well.

Where do I apply the limits in the webui?

@funzoneq

This comment has been minimized.

Copy link
Author

funzoneq commented Apr 18, 2019

@murrant can you review? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.