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

Add Radwin OS Wireless Sensors #8372

Merged
merged 6 commits into from Mar 17, 2018

Conversation

Projects
None yet
6 participants
@vivia11
Contributor

vivia11 commented Mar 14, 2018

Hello,

Adding wireless discovery for distance, tx/rx power and RSSI.
I've tested with Radwin 5000 base and subscription units.

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

@CLAassistant

This comment has been minimized.

CLAassistant commented Mar 14, 2018

CLA assistant check
All committers have signed the CLA.

@MrMaus13

This comment has been minimized.

Contributor

MrMaus13 commented Mar 14, 2018

Works great;
Tested with RW-2250-B350, RW-5200-2250, RW-5520-2150, RW-5050-2250, RW-5550-2150.

@tracernz

This comment has been minimized.

Contributor

tracernz commented Mar 15, 2018

It would be nice to include some test data to ensure this doesn't get broken in future. Info at https://docs.librenms.org/#Developing/os/Test-Units/

@laf

This comment has been minimized.

Member

laf commented Mar 15, 2018

Yes please, test data needed :)

@MrMaus13

This comment has been minimized.

Contributor

MrMaus13 commented Mar 15, 2018

Not sure how this works, but why the general failure?

./scripts/collect-snmp-data.php -h 1322 -m os
OS: radwin
Module(s): os

Capturing Data: Error in packet
Reason: (genError) A general failure occured
Failed object: SNMPv2-SMI::enterprises.12148.10.2.6.0

Error in packet
Reason: (genError) A general failure occured
Failed object: snmpEngineTime.0

Error in packet
Reason: (noSuchName) There is no such variable name in this MIB.
Failed object: hrSystemUptime.0

 sysDescr.0 sysObjectID.0 sysName.0 SNMPv2-MIB::sysDescr.0 SNMPv2-MIB::sysObjectID.0 .1.3.6.1.4.1.12148.10.2.6.0 sysUpTime.0 sysLocation.0 sysContact.0 snmpEngineTime.0 hrSystemUptime.0 1.3.6.1.4.1.4458.1000.1.1.30.0 1.3.6.1.4.1.4458.1000.1.1.3.0 1.3.6.1.4.1.4458.1000.1.1.2.0 1.3.6.1.4.1.4458.1000.1.1.29.0

Updated snmprec data /opt/librenms/tests/snmpsim/radwin.snmprec

Verify this file does not contain any private data before submitting!
@laf

This comment has been minimized.

Member

laf commented Mar 15, 2018

Those are just because the OIDs we are asking for don't exist. Ignore them, if it's created the file then that's all you need.

@vivia11

This comment has been minimized.

Contributor

vivia11 commented Mar 15, 2018

Added in test data for the three different versions I have access to.

@tracernz

This comment has been minimized.

Contributor

tracernz commented Mar 15, 2018

Looks good. 👍 If you run ./scripts/save-test-data.php -o example-os -m <module> it should do a discovery and poll with that test data, and save the discovery and polling results into /tests/data/ . As I understand it that completes the loop so your discovery/polling will always be tested in CI to ensure the results don't change.

@vivia11

This comment has been minimized.

Contributor

vivia11 commented Mar 15, 2018

@tracernz
I'm having trouble with snmpsim on my machine, so I can't generate the json. Is it possible for the json to be generated by someone else from my snmprec files?

@laf

This comment has been minimized.

Member

laf commented Mar 15, 2018

Ok, i've generated the json files. Hopefully travis passes then we can merge in.

@laf laf added Device 🖥 and removed Needs Tests 🦄 labels Mar 15, 2018

@tracernz

This comment has been minimized.

Contributor

tracernz commented Mar 15, 2018

I wonder if something is missing from tests/module_tables.yaml because those files are all empty.
-e-
It already has this, I would think that should be enough..

wireless:
    wireless_sensors:
        excluded_fields: [device_id, sensor_id, access_point_id, lastupdate]
@laf

This comment has been minimized.

Member

laf commented Mar 15, 2018

:( I don't have time to look into this further right now.

@tracernz

This comment has been minimized.

Contributor

tracernz commented Mar 16, 2018

I gave this a try... the data appears to be discovered/polled correctly and inserted into the DB... but nothing comes out.. https://gist.github.com/tracernz/e0b8d23655e6323b7e958da6b9a8a784

And so I tried a different device/module on this branch... and it gave an empty array also. So I checked out master from a few days ago(bbd16dd) and it worked with that device/module... Then I checked out current master (69bedd0) and no test data! It's the master branch that's broken and it broke within the last 5 days.

@tracernz tracernz referenced this pull request Mar 16, 2018

Merged

Fix save-test-data #8399

1 of 1 task complete
@tracernz

This comment has been minimized.

Contributor

tracernz commented Mar 16, 2018

With #8399 merged on top of this branch I captured the data. Can't push to your branch though so here's the data: https://gist.github.com/tracernz/d5467c050e2f779921458966628c25b4
Commands used to capture:

./scripts/save-test-data.php -o radwin -v 5550-0h50 -m wireless
./scripts/save-test-data.php -o radwin -v 5550-0150 -m wireless
./scripts/save-test-data.php -o radwin -v 5200-0150 -m wireless
@scrutinizer-notifier

This comment has been minimized.

scrutinizer-notifier commented Mar 16, 2018

The inspection completed: 4 updated code elements

@laf

laf approved these changes Mar 17, 2018

LGTM

@laf

This comment has been minimized.

Member

laf commented Mar 17, 2018

Thanks @vivia11 (and @tracernz)

@laf laf merged commit 281e01d into librenms:master Mar 17, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@vivia11 vivia11 deleted the vivia11:os-radwin branch Mar 19, 2018

inetAnt added a commit to criteo-forks/librenms that referenced this pull request Mar 19, 2018

device: Added Radwin OS Wireless Sensors (librenms#8372)
* Radwin OS added.

* Changing whitespace.

* Added test data for radwin 5550-0h150,5550-0150, 5200-0150

* Added radwin json test data

* Adding json test data for Radwin

@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.