Closed dilruacs closed 5 years ago
This might happen if you used a configuration which existed before the uuid field has been added. It has been added in this commit: https://github.com/alexmohr/sonyapilib/commit/4f1e63ce6fad95888ea9b675d5011deafd562c6e ( 2 days ago ). This would explain the other issue you opened ( and I already closed again ) I would not implement something which updates the json config with new field because this only happens at development time.
@alexmohr Why not? So you limit yourself in the future to these variables. It could be done in a few lines.
Because it adds unnecessary complexity for something which is most likely not needed. One other field is necessary for the device is necessary which is one for the psk. One valid approach would be to only save necessary fields to the json:
All other data can be read from the device at startup but I rather persist other fields as well ( as it is today )
The current approach does not require to know which fields are relevant for the authentication and communication with the device and I want to keep it that way.
If the remote commands aren't persisted, there is no way to know what command needs to be send to power it on (if this is possible at all, I only have a WOL capable device)
On the other hand: if a future version of the api requires a new field to be persisted, it would be great if this is possible without deleting the json and "pairing" the device from scratch.
wouldn't it be better to just store the API version number instead of a "is_v4" flag?
I've updated the comment above to reflect your answer. Implementing this is currently not on my agenda because I believe it won't be necessary because after finishing the v4 and psk authentication new fields will probably only added for api version 5 ( if sony releases something like that ). Altough I might be wrong and we'll need more fields before that. Keep in mind that v4 is currently still a development branch and I cannot guarantee not to break things because I'm not testing every commit with a device. This will be done before this branch is merged on dev. But I am very happy about the feedback.
The main problem I have with this idea is that the serialized json still would not contain a newly added field and would be initalized with some kind of default value. This would work with things like the uuid but must values I can think of will a value which depends on something meaning if this is the case some kind of configuration file migration must be done anyways.
If any of you has a good idea on how to implement that and account for the problems described I'd be happy to accept a PR. This should also contain a test validating that the serialized data contains all valid fields. Take a look at the previous comment by me to see the necessary values.
wouldn't it be better to just store the API version number instead of a "is_v4" flag?
Yes it would be, I'm going replace the flag with the version number.
My idea was that you load the device instance from the json (jsonpickle). Then you create a new SonyDevice instance with host and nickname from the loaded instance. Compare the attributes of both instances and copy the missing ones from the new instance to the loaded one. So you cover all attributes that will be set in the init
If the attributes are compared without the need to list them explicitly this could be an elegant solution. But it would lead to running the init_device
method at every startup, it should be made sure that this works if the device is not powered on.
If a difference is detected the stored json must be updated because, in this uuid example, a new uuid would be generated each time leading to breaking the authentication.
Beside that even if this uuid would have been persisted in the json the pairing had to done again because when I added this I also changed how the device id is generated meaning the sony device wouldn't be able to match it with the paired configuration due to the id missmatch and a new pairing is neccessary anyway.
If you are able to implement this in "a few lines" go ahead but I cannot come up with a scenario where this is really neccessary ;)
I have a fresh copy of the v4 branch.
I tried to power on the device and got the following stacktrace:
https://github.com/alexmohr/sonyapilib/blob/0d8548ed5fa22b574016e5d09822b35d613a8da3/sonyapilib/device.py#L99 At this point the attribute should be there, I do not understand why the error occurs.