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

Add support of Hirschmann switch #450

Merged
merged 10 commits into from Feb 20, 2023
Merged

Add support of Hirschmann switch #450

merged 10 commits into from Feb 20, 2023

Conversation

Infopromt
Copy link
Contributor

No description provided.

Copy link
Member

@JeroenvIS JeroenvIS left a comment

Choose a reason for hiding this comment

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

Looks like a nice start!

A couple of things that you might want to address or clarify:

  • There are some references to "3Com" in the file, you probably used that as a starting point? I suggest that you change those.
  • Is there a specific reason to define ''h_serial_number'' with a numeric OID? Where possible we prefer to load the right MIBs and resolve the OID via leaf name.
  • Can you please put in some comments around the ''model'' logic? Without comments, it doesn't make sense that the model is 'RS40-30' if $model is undefined or if $model contains 'rs30'.
  • Is there a specific reason to override the mac() method? For a layer 2 device, I would expect the one from Layer2.pm / Bridge.pm would work. Just curious.
  • If the mac() override is indeed needed, please document it in the POD section.

@Infopromt
Copy link
Contributor Author

I've made the needed corrections. I hope that will be enough

@ollyg ollyg merged commit 1d4cd2d into netdisco:master Feb 20, 2023
@ollyg
Copy link
Member

ollyg commented Feb 20, 2023

thank you @Infopromt and @JeroenvIS !
merged and will be in next release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants