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

New os Alcoma #7476

Merged
merged 3 commits into from
Oct 17, 2017
Merged

New os Alcoma #7476

merged 3 commits into from
Oct 17, 2017

Conversation

erotel
Copy link
Contributor

@erotel erotel commented Oct 11, 2017

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

@laf
Copy link
Member

laf commented Oct 15, 2017

Thanks @erotel. All looks good, just a question on the os name. It's usually not a good idea to base it on the vendor name. Looking at the firmware for this device it doesn't appear to be called anything but to safeguard the vendor building a new product line should we use alcoma-al?

@laf laf added the Device 🖥️ New or added device support label Oct 15, 2017
@erotel
Copy link
Contributor Author

erotel commented Oct 15, 2017

this device does not support ssh or telnet, so I do not know the system name.
The firmware is the same for both the AL series and the MP series.
the older ALxxMP / ME series (MPS, ME, ME32, MP90) does not support snmp queries wirelessly.
Can I renaming it to "Alcoma_almp"?

@laf
Copy link
Member

laf commented Oct 15, 2017

@erotel seems fine with me, alcoma-almp though to be consistent.

@@ -0,0 +1,97 @@
<?php
/**
* Alcoma.php
Copy link
Member

Choose a reason for hiding this comment

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

For completeness may as well change this

@scrutinizer-notifier
Copy link

The inspection completed: 2 new issues, 5 updated code elements

@laf laf merged commit 6e379bf into librenms:master Oct 17, 2017
@lock
Copy link

lock bot commented May 17, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

@lock lock bot locked as resolved and limited conversation to collaborators May 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Device 🖥️ New or added device support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants