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

Improve Ubnt serial and SSID detection #657

Merged
merged 13 commits into from
Apr 24, 2024

Conversation

eduardomozart
Copy link
Contributor

Show SSID names instead of radio ifname on GLPI UI and fix Serial getting on UniFi AP series.

Show SSID names instead of radio ifname on GLPI UI and fix Serial getting on UniFi AP series.
Copy link
Member

@g-bougard g-bougard left a comment

Choose a reason for hiding this comment

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

Hi @eduardomozart

thank you for your contribution. It seems interesting for UBnT support.

Can you check my comments and change your code accordingly ?

It would be nice if you can also provide a full snmpwalk in the expected format for testing.

lib/GLPI/Agent/SNMP/MibSupport/Ubnt.pm Outdated Show resolved Hide resolved
lib/GLPI/Agent/SNMP/MibSupport/Ubnt.pm Show resolved Hide resolved
lib/GLPI/Agent/SNMP/MibSupport/Ubnt.pm Outdated Show resolved Hide resolved
lib/GLPI/Agent/SNMP/MibSupport/Ubnt.pm Outdated Show resolved Hide resolved
lib/GLPI/Agent/SNMP/MibSupport/Ubnt.pm Outdated Show resolved Hide resolved
@g-bougard g-bougard changed the title Improve serial and SSID detection Improve Ubnt serial and SSID detection Apr 22, 2024
eduardomozart and others added 2 commits April 22, 2024 13:35
Co-authored-by: Guillaume Bougard <gbougard@teclib.com>
Copy link
Member

@g-bougard g-bougard left a comment

Choose a reason for hiding this comment

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

Hi @eduardomozart

thank you for the changes.

I still see few things to update.

So can you check my comments ? One is just a question you must clarify.

Also can you provide the snmpwalk I requested so I can include it in our private tests and validate this PR works as expected ?

lib/GLPI/Agent/SNMP/MibSupport/Ubnt.pm Outdated Show resolved Hide resolved
lib/GLPI/Agent/SNMP/MibSupport/Ubnt.pm Outdated Show resolved Hide resolved
lib/GLPI/Agent/SNMP/MibSupport/Ubnt.pm Outdated Show resolved Hide resolved
lib/GLPI/Agent/SNMP/MibSupport/Ubnt.pm Show resolved Hide resolved
lib/GLPI/Agent/SNMP/MibSupport/Ubnt.pm Outdated Show resolved Hide resolved
lib/GLPI/Agent/SNMP/MibSupport/Ubnt.pm Outdated Show resolved Hide resolved
Co-authored-by: Guillaume Bougard <gbougard@teclib.com>
@eduardomozart
Copy link
Contributor Author

eduardomozart commented Apr 23, 2024

Hi @eduardomozart

thank you for the changes.

I still see few things to update.

So can you check my comments ? One is just a question you must clarify.

Also can you provide the snmpwalk I requested so I can include it in our private tests and validate this PR works as expected ?

Here's the snmpwalk output as you requested:

u6-lite.walk.txt

Some Radios will not be reported with their respective names (will still be shown as "raX" or "raiX") as there's some Radios which are used internally for Mesh and Discovery, so there's no SSID associated to them, but user created SSID will report their names as expected. There also seems that Ubiquiti reports those iftypes as 6 (Ethernet) on OID but I believe they should be iftype as 188 (Wifi) as Aruba AP reports them as such, but GLPI doesn't import those kind of interfaces by default (I'll open ) and not sure if it would be OK to replace the OID iftype of Ubiquiti devices, but it would provide more information that the sysadmin could provide related to those radios, like "Wifi protocol version" and "Wifi mode", and also associate a "Wifi network", but GLPI doesn't seem to support importing 188 "iftypes" by default, so on the near future when I start working on a similar patch for Aruba devices and already reported such bug on glpi-project/glpi#16981

Edit: I was wrong about get*ByMibSupport calls: when they return empty, the existent Device value isn't replaced by empty ones. One example would be to edit getMacAddress sub and remove the ${device}->{MAC} and the device still would report MAC address as expected, so I refactor the code a little bit. Thanks for your suggestions, I'm learning a lot about Perl and code guidelines following your guidance, sorry for my mistakes as it's the first time I'm programming in Perl at all.

eduardomozart and others added 3 commits April 23, 2024 09:01
Co-authored-by: Guillaume Bougard <gbougard@teclib.com>
Co-authored-by: Guillaume Bougard <gbougard@teclib.com>
@eduardomozart
Copy link
Contributor Author

I removed the whitespaces and replaced the default iftype so the Radio interfaces are reported as WiFi as discussed in glpi-project/glpi#16981 so the sysadmin could provide related to those radios, like "Wifi protocol version" and "Wifi mode", and also associate a "Wifi network".

Copy link
Member

@g-bougard g-bougard left a comment

Choose a reason for hiding this comment

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

Hi @eduardomozart

sorry, I missed few needed checks yesterday. Nothing really critical, but this is just to make your code perfect.

Another point, packaging building is failing and I've included fixes in develop branch. Can you rebase your branch to include that GH Actions fixes ?

lib/GLPI/Agent/SNMP/MibSupport/Ubnt.pm Outdated Show resolved Hide resolved
lib/GLPI/Agent/SNMP/MibSupport/Ubnt.pm Outdated Show resolved Hide resolved
eduardomozart and others added 3 commits April 24, 2024 07:47
Co-authored-by: Guillaume Bougard <gbougard@teclib.com>
Co-authored-by: Guillaume Bougard <gbougard@teclib.com>
@eduardomozart
Copy link
Contributor Author

Hi @eduardomozart

sorry, I missed few needed checks yesterday. Nothing really critical, but this is just to make your code perfect.

Another point, packaging building is failing and I've included fixes in develop branch. Can you rebase your branch to include that GH Actions fixes ?

Hello, I've applied your changes and tested them on production, they seem to be working fine. Also rebase my repo.

@g-bougard g-bougard merged commit 081728c into glpi-project:develop Apr 24, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants