[freeboxos] Fix enabling/disabling of Mac OS file sharing

[lolodomo]

Fix #17200

Signed-off-by: Laurent Garnier lg.hc@free.fr

[lsiepel]

Replace an enum with a string? I would opt to change the configuration parameter to have options in the xml, so the value can be chosen from the UI.

For file based config, I propose to add a check in the initialize method. This check would check if the enum.valueOf(config.parameter.topUpper()) throws an illegal argument exception and set the thing state to configuration_error accordingly.

string is also possible, but with a limited list I would really prefer enum.

[lolodomo]

This is only an API field that is returned by the API and just provided to the API when changing the config.
I could also remove it but @clinique option was to have a full DTO which makes also sense.

[lolodomo]

To be clear, we need to keep the raw value returned by the get API to then provide it when calling the set API.

[lolodomo]

Look at lines 61-63 to understand.
https://github.com/openhab/openhab-addons/blob/main/bundles/org.openhab.binding.freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/api/rest/AfpManager.java#L62

It is not OH thing configuration but Freebox OS config REST API.

[lsiepel]

Based on the text in the linked issue I thought it was the openHAB config. Nevertheless still prefer proper enum conversion, no magic strings in the code, but it ie much less important and certainly not blocking.

LGTM

[lolodomo]

API filed is a string.

[clinique]

Still I don't understand the purpose of removing the enum.

[lolodomo]

Still I don't understand the purpose of removing the enum.

The get API returns value "airport" in lowercase for the field server_type.
When using the set API, you have to use "airport" also. If the field is set with "AIRPORT", the API simply fails.
Due to the conversion to an enum, string "airport" is first converted to enum value AIRPORT when handling the get API response. When reused and serialized in the PUT request, the server_type field was set with "AIRPORT" and the API failed.

@clinique : is it clearer now ?

Edit: look at what you did in setStatus method.