Skip to content

Fix smartos-dcp-m.yaml#15295

Merged
Jellyfrog merged 31 commits intolibrenms:masterfrom
electrocret:Fix-Smartos-dcp-m
Sep 12, 2023
Merged

Fix smartos-dcp-m.yaml#15295
Jellyfrog merged 31 commits intolibrenms:masterfrom
electrocret:Fix-Smartos-dcp-m

Conversation

@electrocret
Copy link
Copy Markdown
Member

@electrocret electrocret commented Sep 7, 2023

Appears to have been a typo

Please give a short description what your pull request is for

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.

@murrant
Copy link
Copy Markdown
Member

murrant commented Sep 8, 2023

Can you add skip_value: -99 (or -990 whatever it is)? This many sensors for ports without modules is ridiculous :D

@electrocret
Copy link
Copy Markdown
Member Author

electrocret commented Sep 8, 2023

Can you add skip_value: -99 (or -990 whatever it is)? This many sensors for ports without modules is ridiculous :D

Yeah.... I think the original committer used a device in a lab for test units, and it didn't actually have anything connected to it. Probably how the typo got overlooked. Once I fixed it data popped right in for my equipment.

@electrocret electrocret marked this pull request as ready for review September 8, 2023 14:35
@electrocret
Copy link
Copy Markdown
Member Author

electrocret commented Sep 8, 2023

I'm actually ready for review with this PR now.

Thought I'd do some prettying up since I was tinkering with it.

Edit- I lied. Someone on my team pointed out another issue with multi line card models.

@electrocret electrocret added the Device 🖥️ New or added device support label Sep 8, 2023
@electrocret
Copy link
Copy Markdown
Member Author

electrocret commented Sep 8, 2023

Alrighty, now I'm done.

Edit - I lied again. The Sensor Grouping was making my brain itch cause it was wholly unnecessary since I didn't find much else in the MIB worth tracking.

@electrocret
Copy link
Copy Markdown
Member Author

electrocret commented Sep 11, 2023

Alrighty, this PR is ready. Someone please approve and merge before I find something else to nit pick about it. @murrant @Jellyfrog @VVelox 😄

Copy link
Copy Markdown
Contributor

@VVelox VVelox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, looks good to me. I am not seeing anything that looks crazy or the like off hand, but I'm by far not the most familiar with this area of discovery.

So likely would be good to have some one take a second peak at it.

Copy link
Copy Markdown
Contributor

@PipoCanaja PipoCanaja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM indeed, as far as I can tell without owning a device :)

@Jellyfrog Jellyfrog merged commit a1eb90f into librenms:master Sep 12, 2023
@electrocret electrocret deleted the Fix-Smartos-dcp-m branch September 12, 2023 11:36
@librenms-bot
Copy link
Copy Markdown

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

https://community.librenms.org/t/23-9-0-changelog/22320/1

peejaychilds pushed a commit to peejaychilds/librenms that referenced this pull request Oct 26, 2023
* Update smartos-dcp-m.yaml

* Update smartos-dcp-m_dcp-m40-pam4-zr.json

* Update smartos-dcp-m_dcp-m40-pam4-zr.json

* Update smartos-dcp-m_dcp-m40-pam4-zr.json

* Update smartos-dcp-m_dcp-m40-pam4-zr.json

* Update smartos-dcp-m_dcp-m40-pam4-zr.json

* Update smartos-dcp-m_dcp-m40-pam4-zr.json

* Update smartos-dcp-m.yaml

* Update smartos-dcp-m.yaml

* Update smartos-dcp-m_dcp-m40-pam4-zr.json

* Remove bogus 0 values

* Update smartos-dcp-m.yaml

* Update smartos-dcp-m.yaml

* Update smartos-dcp-m.yaml

* Update smartos-dcp-m.yaml

* Update smartos-dcp-m.yaml

* Update DCP-INTERFACE-MIB

* Update SO-TC-MIB

* Update smartos-dcp-m.yaml

* Update smartos-dcp-m.yaml

* Update smartos-dcp-m.yaml

* Update smartos-dcp-m.yaml

* Update smartos-dcp-m.yaml

* Update smartos-dcp-m.yaml

* Update smartos-dcp-m.yaml

* Update smartos-dcp-m.yaml

* Test data update

* Update smartos-dcp-m.yaml

* Update smartos-dcp-m_dcp-m40-pam4-zr.json

* Remove Grouping

* Update Test
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.

6 participants