UniFi-API-client icon indicating copy to clipboard operation
UniFi-API-client copied to clipboard

change create_wlan function

Open OrionTheGiant opened this issue 2 years ago • 10 comments

No longer requires wlangroup_id and API endpoint updated.

I don't know how you'd like to handle having two different API endpoints based on the controller version so I just updated the existing line to point to the new endpoint. Happy to add some additional logic to check controller version if there's already a standard way of checking that.

OrionTheGiant avatar Aug 22 '22 17:08 OrionTheGiant

Could you share how you use the create_wlan function? After implementing the proposed changes, I am getting an invalid payload message.

karpiatek avatar Aug 22 '22 22:08 karpiatek

Thanks for sharing this. I need to think about how to make this backwards compatible, e.g. by checking against the (controller) version property. Do we know with which controller version this behavior changed?

Also, we currently don't yet use the (controller) version property so need to implement this in a smart and re-usable manner.

malle-pietje avatar Aug 23 '22 07:08 malle-pietje

Am I correct in thinking the switchover when the /add/wlanconf was removed occurred with the introduction of version 6.X?

In that case the /rest/wlanconf route already existed with version 5.X so we could just modify the method/function as committed in this PR and use the comments to instruct devs about the change in behavior.

malle-pietje avatar Aug 23 '22 07:08 malle-pietje

@karpiatek I didn't change the function signature aside from setting wlangroup_id to default null so it shouldn't be any different. Are you including the array of ap_group_ids? Only asking because I missed that the first time around too.

$name = 'APIWiFi';
$password = 'APIWiFiPassword';
$usergroup_id = 'xxxxxxxxxxxxxxxxxx';

$result = $unifi_connection->create_wlan($name, $password, $usergroup_id, ...);

OrionTheGiant avatar Aug 23 '22 11:08 OrionTheGiant

@malle-pietje I'm not sure which version this changed in. Do you have a way to check that other than pulling different versions of the controller and snooping on the API call? This seems like a major version sort of change so 6.X or 7.X would be the likely candidates. 6.X would seem to make sense since (according to the create_wlan parameter comments) that's also when ap_group_ids became a required parameter

As for handling different endpoints, a utility function for checking greater/less than for the version number string will be helpful and then you can do something like

$endpoint = ($this->version_greater_than('6.0')) ? 'rest/wlanconf' : 'add/wlanconf';
return $this->fetch_results_boolean('/api/s/' . $this->site . $endpoint,  $payload);

OrionTheGiant avatar Aug 23 '22 11:08 OrionTheGiant

@OrionTheGiant Yes, I do. I don't know, maybe I missed something.

<?php

require_once 'vendor/autoload.php';
require_once 'config.php';

$site_id ='iexxxxat';

$name ='APITest';
$x_passphrase ='Test1234#';
$usergroup_id ='62fcxxxxxxxxxxxxxxxxdef4';
$vlan_id = '62fcxxxxxxxxxxxxxxxx4b40';
$enabled = true;
$hide_ssid = false; 
$is_guest = false; 
$security = 'wpapsk'; 
$wpa_mode = 'wpa2'; 
$wpa_enc = 'ccmp'; 
$uapsd_enabled = false; 
$schedule = [];
$schedule_enabled = false;
$ap_group_ids = '62fexxxxxxxxxxxxxxxx9790'; 

$unifi_connection = new UniFi_API\Client($controlleruser, $controllerpassword, $controllerurl, $site_id, $controllerversion);
$unifi_connection->set_debug(true);
$loginresults     = $unifi_connection->login();
$results          = $unifi_connection->create_wlan($name, $x_passphrase, $usergroup_id, $wlangroup_id, $enabled, $hide_ssid, $is_guest, $security, $wpa_mode, $wpa_enc, $vlan_enabled, $vlan_id, $uapsd_enabled, $schedule_enabled, $schedule, $ap_group_ids);
$debug		  = $unifi_connection->get_debug();

echo json_encode($results, JSON_PRETTY_PRINT);
echo json_encode($debug, JSON_PRETTY_PRINT);

I'm running UniFi version 6.x and I'm pretty sure, that if the changes you made work in version 7.x, it should also work in version 6.x. Apparently, I made some kind of mistake here. The code in Client.php is exactly the same as yours. I'm sorry for spamming here, it's just an engineer's curiosity. Trying to figure out what's wrong ;)

karpiatek avatar Aug 24 '22 18:08 karpiatek

@karpiatek I don't know if this is your only problem but the UniFi API needs ap_group_ids to be an array of strings, not just a string:

$ap_group_ids = ['62fexxxxxxxxxxxxxxxx9790'];

I ran into the same thing the first time trying create_wlan so I just added an additional check to add the ap_group_ids to an array if a string is received as input so it shouldn't be a problem anymore

OrionTheGiant avatar Aug 24 '22 18:08 OrionTheGiant

I had similar observations while working on #191. In addition to the changes specifically to the wlangroup_id parameter, I think a clear versioning of this library would be useful and my recommendation would be to match the version of the controller itself (like in elastic/elasticsearch-php any many other client libraries). This approach makes it easier to follow changes without having to hack around older versions for backwards compatibility.

sgrodzicki avatar Apr 16 '23 18:04 sgrodzicki

I had similar observations while working on #191. In addition to the changes specifically to the wlangroup_id parameter, I think a clear versioning of this library would be useful and my recommendation would be to match the version of the controller itself (like in elastic/elasticsearch-php any many other client libraries). This approach makes it easier to follow changes without having to hack around older versions for backwards compatibility.

I see what you mean but this cannot be handled through versioning of the API client class. For example, most of our software packages are multi-site, multi-controller meaning that a single software instance must be able to talk with large number of controller versions. One of our instances actually supports almost each controller version between 5.11.50 and 7.3.83…

It makes more sense IMHO to have the API client deal with such changes which is why we still have the $controller_version property. It is still not used but I’m open to suggestions on how to implement this.

One of the next challenges will be to document this in a clear manner.

malle-pietje avatar Apr 17 '23 06:04 malle-pietje

I had similar observations while working on #191. In addition to the changes specifically to the wlangroup_id parameter, I think a clear versioning of this library would be useful and my recommendation would be to match the version of the controller itself (like in elastic/elasticsearch-php any many other client libraries). This approach makes it easier to follow changes without having to hack around older versions for backwards compatibility.

I see what you mean but this cannot be handled through versioning of the API client class. For example, most of our software packages are multi-site, multi-controller meaning that a single software instance must be able to talk with large number of controller versions. One of our instances actually supports almost each controller version between 5.11.50 and 7.3.83…

It makes more sense IMHO to have the API client deal with such changes which is why we still have the $controller_version property. It is still not used but I’m open to suggestions on how to implement this.

One of the next challenges will be to document this in a clear manner.

Fair enough. I was not aware of such use cases. Then I suggest to start by introducing unit tests and integration tests (in combination with a PHP version matrix). I can work on this.

sgrodzicki avatar Apr 17 '23 07:04 sgrodzicki