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

Added support for EDFA #9912

Merged
merged 6 commits into from Mar 27, 2019

Conversation

Projects
None yet
5 participants
@jozefrebjak
Copy link
Contributor

commented Mar 6, 2019

fixes #9826

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.

jozefrebjak added some commits Mar 6, 2019

@TheGreatDoc

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2019

@jozefrebjak

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

@TheGreatDoc ok i will try to add oid of my edfa to this discovery and i will see what happen.

@jozefrebjak

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

@TheGreatDoc ok its almost the same, but you are using low and high limits, and it all done with php, but i am not getting all of the information i believe with #9125 here it is how its look like when i deleted .1 from discovery and used only .1.3.6.1.4.1.17409.1
image

and this how its look like with my PR

image

We dont have anything connected for now, it is in a testing stage. So i dont know if i need to remove this PR and use what is done or we rebuild it little bit.

jozefrebjak added some commits Mar 6, 2019

@jozefrebjak jozefrebjak referenced this pull request Mar 6, 2019

Closed

New Device : EDFA SNMP Agent V2.0 #9826

0 of 3 tasks complete
@cppmonkey

This comment has been minimized.

Copy link
Contributor

commented on includes/definitions/discovery/edfa.yaml in de39582 Mar 6, 2019

Might want to check https://docs.librenms.org/Developing/Sensor-State-Support/

PowerUp is an error state?

I'd just go through the other state 'generic' values.

Other than that. Looking good!

@romsworld

This comment has been minimized.

Copy link

commented Mar 12, 2019

Hi,

Thanks a lot, I will test next week.

@romsworld

This comment has been minimized.

Copy link

commented Mar 12, 2019

Hi,

For OutputOpticalPower, could you change descr : Total Output Optical Power > Output Optical Power

For BIAS 1, I have 8600A, but i think the real value is 86mA.

Is ti possible to have green or red value for dbm ?

@TheGreatDoc

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2019

Is ti possible to have green or red value for dbm ?

You set this up in Device -> Edit -> Health. Set the high/low values.

@romsworld

This comment has been minimized.

Copy link

commented Mar 19, 2019

Hi,
Everything seems okay to me.
Is it necessary to do another test before going into production?

Thank you.

@murrant murrant changed the title WIP Added support for EDFA Added support for EDFA Mar 27, 2019

@murrant murrant added the Device 🖥 label Mar 27, 2019

@murrant murrant merged commit 54893b9 into librenms:master Mar 27, 2019

1 of 2 checks passed

codeclimate Code Climate encountered an error attempting to analyze this pull request.
Details
license/cla Contributor License Agreement is signed.
Details
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.