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

Fixing memory scale for datacom-dmos devices #15640

Merged
merged 11 commits into from Dec 21, 2023

Conversation

lhwolfarth
Copy link
Contributor

@lhwolfarth lhwolfarth commented Dec 14, 2023

Fixing memory scale for datacom-dmos devices

DO NOT DELETE THE UNDERLYING TEXT

Please note

Please read this information carefully. You can run ./lnms dev:check to check your code before submitting.

  • Have you followed our code guidelines?
  • If my Pull Request does some changes/fixes/enhancements in the WebUI, I have inserted a screenshot of it.
  • If my Pull Request makes discovery/polling/yaml changes, I have added/updated test data.

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.

Fixing memory scale for datacom-dmos devices
@kkrumm1 kkrumm1 added the Device 🖥️ New or added device support label Dec 15, 2023
@PipoCanaja
Copy link
Contributor

According to the tests, it seems that it is not working. Moreover, you fixed the index to '1'. Better to use the index provided by the device in the MIB (that would allow the device to provide more than one value).
Does this code work in your environnement ? If yes, then you probably need to upgrade the SNMPREC file (and match the JSON file to it of course)

@PipoCanaja PipoCanaja added the Needs Tests 🦄 https://docs.librenms.org/Developing/os/Test-Units/ label Dec 16, 2023
Copy link

Please add test data so we can ensure your change is not broken in the future.
Read the docs to find out how: https://docs.librenms.org/Developing/os/Test-Units

@lhwolfarth
Copy link
Contributor Author

lhwolfarth commented Dec 18, 2023

@PipoCanaja, in my tests it was working. I also tested now without using the index 1 and it changed from the scale from MiB to GiB but still consistent (right values).
I will update the yaml removing the "index 1" and then upload the new test data.
Thank you!

lhwolfarth and others added 10 commits December 18, 2023 09:26
Removing the line "index: 1" because the MIB shall provide the existent indexes.
Uploading updated snmprec files after changing the datacom-dmos.yaml in order to fix the memory scale.
Uploading updated json files after changing the datacom-dmos.yaml in order to fix the memory scale.
Updated datacom-dmos test files (snmprec and json). I had problems generating these files, so please ignore the commits without files I did earlier.
@PipoCanaja
Copy link
Contributor

Seems that the tests went somehow wrong (all empty). Fixing that for you

@PipoCanaja PipoCanaja removed the Needs Tests 🦄 https://docs.librenms.org/Developing/os/Test-Units/ label Dec 21, 2023
@PipoCanaja
Copy link
Contributor

LGTM, so let's control the tests and it will be ready for merge

@PipoCanaja PipoCanaja added this to the 23.12.0 milestone Dec 21, 2023
@PipoCanaja PipoCanaja merged commit 159b565 into librenms:master Dec 21, 2023
9 checks passed
@lhwolfarth lhwolfarth deleted the datacom-dmos-memory-scale-fix branch December 22, 2023 12:10
@librenms-bot
Copy link

This pull request has been mentioned on LibreNMS Community. There might be relevant details there:

https://community.librenms.org/t/24-1-0-changelog/23271/1

gunkaaa pushed a commit to gunkaaa/librenms that referenced this pull request Jan 8, 2024
* Fixing memory scale for datacom-dmos devices

Fixing memory scale for datacom-dmos devices

* Update datacom-dmos.yaml

Removing the line "index: 1" because the MIB shall provide the existent indexes.

* Uploading updated snmprec files

Uploading updated snmprec files after changing the datacom-dmos.yaml in order to fix the memory scale.

* Uploading updated json files

Uploading updated json files after changing the datacom-dmos.yaml in order to fix the memory scale.

* datacom-dmos updated snmprec files

* snmprec files

* Updated datacom-dmos test files (snmprec and json)

Updated datacom-dmos test files (snmprec and json). I had problems generating these files, so please ignore the commits without files I did earlier.

* Fix Tests

---------

Co-authored-by: PipoCanaja <38363551+PipoCanaja@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Device 🖥️ New or added device support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants