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 Zmtel greenpacket devices (os: zmtel) #10067

Merged
merged 6 commits into from Apr 29, 2019

Conversation

Projects
None yet
5 participants
@spencerbutler
Copy link
Contributor

commented Apr 4, 2019

Fixes:
#10065
#8729

I'm not entirely sure about the Frequencies. The device reports in KHz and LibreNMS uses Hz. I multiplied by 1000, but it doesn't seem correct. Please advise.

ulFrequency OBJECT-TYPE
SYNTAX OCTET STRING (SIZE (0..16))
MAX-ACCESS read-only
STATUS current
DESCRIPTION "Indicates the mobile network uplink frequency(KHz)."
::= { lteInfo 9 }

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.

@spencerbutler spencerbutler referenced this pull request Apr 4, 2019

Open

New Device: Greenpacket OT-350 #10065

0 of 3 tasks complete
@inkweb

This comment has been minimized.

Copy link

commented Apr 4, 2019

So i did "./scripts/github-apply 10067" as the librenms user, but the devices are still showing up as "generic device"

@PipoCanaja

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

@inkweb

rm cache/os_defs.cache 

That will ensure the new patch is used.

@inkweb

This comment has been minimized.

Copy link

commented Apr 5, 2019

That worked, thanks.

So for the frequencies, here's what LibreNMS is reporting:

Bandwidth 20000000Hz
Download 3612500000Hz
Upload 3612500000Hz

And this is what the CPE itself says:

DL Frequency 3612500 KHz
UL Frequency 3612500 KHz
Bandwidth 20000 KHz

Everything else looks good so far. There are some items missing that we require to monitor the units:

  • SINR
  • CINR0
  • CINR1
  • PCI
  • Dl MCS
  • Ul MCS

If those could be added as well, that would be wonderful.

@spencerbutler

This comment has been minimized.

Copy link
Contributor Author

commented Apr 5, 2019

That worked, thanks.

So for the frequencies, here's what LibreNMS is reporting:

Bandwidth 20000000Hz
Download 3612500000Hz
Upload 3612500000Hz

And this is what the CPE itself says:

DL Frequency 3612500 KHz
UL Frequency 3612500 KHz
Bandwidth 20000 KHz

Everything else looks good so far. There are some items missing that we require to monitor the units:

  • SINR
  • CINR0
  • CINR1
  • PCI
  • Dl MCS
  • Ul MCS

If those could be added as well, that would be wonderful.

Great. I'll fix up the frequencies and see if I can add the requested data points.

@spencerbutler

This comment has been minimized.

Copy link
Contributor Author

commented Apr 5, 2019

@inkweb git this latest commit a try and let me know what you think.

@murrant

This comment has been minimized.

Copy link
Member

commented Apr 7, 2019

@inkweb those are the same frequencies...

@murrant
Copy link
Member

left a comment

This PR is abusing the count sensor.

Please do not do that.

@spencerbutler

This comment has been minimized.

Copy link
Contributor Author

commented Apr 7, 2019

This PR is abusing the count sensor.

Please do not do that.

fixed

@PipoCanaja PipoCanaja added this to the 1.51 milestone Apr 7, 2019

@inkweb

This comment has been minimized.

Copy link

commented Apr 8, 2019

@inkweb those are the same frequencies...

yes i know, i'm just letting everyone know what i'm seeing on this end as there was some question about the frequency handling in the update.

@inkweb

This comment has been minimized.

Copy link

commented Apr 8, 2019

Frequencies, SINR, CINR0 and CINR1 are all looking great, thank you! Any luck with PCI and the two MCS datas?

@spencerbutler

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

Any luck with PCI and the two MCS datas?

Those were moved to the "Operating System" line in the "Overview" section.

@inkweb

This comment has been minimized.

Copy link

commented Apr 8, 2019

Ah i see. Any way to make them graphable? It would help with troubleshooting.

@spencerbutler

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

Do those values change over time? If so, I'll look into the proper way to graph them.

@inkweb

This comment has been minimized.

Copy link

commented Apr 8, 2019

Yes they do change

@laf
Copy link
Member

left a comment

Can you also drop the .txt extension from the mib as well please.

Show resolved Hide resolved includes/polling/os/zmtel.inc.php Outdated

@laf laf added the User-Pending label Apr 9, 2019

@spencerbutler

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2019

Can you also drop the .txt extension from the mib as well please.

fixed

@spencerbutler

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2019

@inkweb I added those MCS values to the state sensor. Please update, test, and provide feedback.

@inkweb

This comment has been minimized.

Copy link

commented Apr 9, 2019

MCS (and everything else so far) seems to be working great. Graphing PCI would be great too, so we can find people tower jumping.

@spencerbutler

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2019

MCS (and everything else so far) seems to be working great. Graphing PCI would be great too, so we can find people tower jumping.

It should be graphing now.

@inkweb

This comment has been minimized.

Copy link

commented Apr 10, 2019

It is graphing, but the graph is kind of odd, with the line pressed against the bottom and the whole scale on the left being a single number.

@inkweb

This comment has been minimized.

Copy link

commented Apr 12, 2019

but its not a deal breaker by any means. basically we're fine with the device as it stands now. thank you for the work!

@spencerbutler

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

but its not a deal breaker by any means. basically we're fine with the device as it stands now. thank you for the work!

Awesome! Glad to hear it's working for you.

@murrant murrant removed the User-Pending label Apr 29, 2019

@murrant murrant merged commit 9cfa2aa into librenms:master Apr 29, 2019

5 of 6 checks passed

codeclimate Code Climate encountered an error attempting to analyze this pull request.
Details
Inspection Summary
Details
Node: analysis
Details
Travis CI - Pull Request Build Passed
Details
WIP Ready for review
Details
license/cla Contributor License Agreement is signed.
Details

funzoneq added a commit to funzoneq/librenms that referenced this pull request Apr 30, 2019

Added support for Zmtel greenpacket devices (os: zmtel) (librenms#10067)
* Added support for Zmtel greenpacket devices (os: zmtel)

* added more sensors

* move count sensors to features

* rename MIB and move modulation scheme to state sensor

* added test data

* graph pCID

spencerbutler added a commit to spencerbutler/librenms that referenced this pull request May 21, 2019

Added support for Zmtel greenpacket devices (os: zmtel) (librenms#10067)
* Added support for Zmtel greenpacket devices (os: zmtel)

* added more sensors

* move count sensors to features

* rename MIB and move modulation scheme to state sensor

* added test data

* graph pCID
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.