Add vm_specific parameter to DoIPClient initialization #60
Conversation
…add 'vm_specific' as an init argument and store it as a static value for the entire instance. Add .idea/ to .gitignore
jacobschaer
left a comment
jacobschaer
left a comment
There was a problem hiding this comment.
Sorry for the delay I forgot to mark the review finished
doipclient/client.py
Outdated
|
|
||
| @property | ||
| def vm_specific(self): | ||
| """Get the optional OEM specific field value if set. | ||
| :return: vm_specific value | ||
| :rtype: int, optional | ||
| """ | ||
| return self._vm_specific | ||
|
|
||
| @vm_specific.setter | ||
| def vm_specific(self, value): | ||
| """Set the optional OEM specific field value. If you do not need to send this item, set it to None. | ||
| :param value: The vm_specific value (must be > 0 and <= 0xffffffff) or None. | ||
| :type value: int, optional | ||
| :raises ValueError: Value is invalid or out of range | ||
| """ | ||
| if not isinstance(value, int) and value is not None: | ||
| raise ValueError("Invalid vm_specific type must be int or None") | ||
| if isinstance(value, int) and (value < 0 or value > 0xffffffff): | ||
| raise ValueError("Invalid vm_specific value must be > 0 and <= 0xffffffff") | ||
| self._vm_specific = value |
There was a problem hiding this comment.
Do we anticipate the user needing to change vm_specific over the life of the object? I don't mind the setter or getter though. What I am wondering though is if we can move the validation out of the setter and into a helper function like validate_vm_specific(value). Since you went to the trouble of validating here, but the user is allowed to pass an unvalidated vlaue to request_activation(). Might as well validate in both places. Otherwise this seems fine
…specific value also in request_activation; add tests
|
Thank you for the great catch. I moved the validation to a static method and added the validation to request_validation(). |
2 participants
Hello,
I add vm_specific parameter also to the
DoIPClientinitialization. The value is now stored as a static attr. of the instance so, whenever in request_activation is called, then value is used, when value is not overwrited by method's argument. This means I can set vm_specific in initialization, and it will be directly used in RouterActivationRequest, including during reconnection.Value is handled by setter and getter - this allows it to be overridden and used as a hook.
It is fully backward compatible.
Feel free to suggest any modifications or refuse whole request.
Thanks,
Jakub