-
Notifications
You must be signed in to change notification settings - Fork 76
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
property won't set if it is read only. #156
base: master
Are you sure you want to change the base?
Conversation
Thing.h
Outdated
@@ -360,6 +360,10 @@ class ThingProperty : public ThingItem { | |||
callback(newValue); | |||
} | |||
} | |||
bool isInRange(ThingDataValue value){ | |||
return (this->maximum < this->minimum) //to check if minimum and maximum values are asigned | |||
|| (this->maximum > value.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why dot in value. ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is isInRange used ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, at first I was going to handle the process of validation the range in a function then decide to do it directly but forget to delete the function. this function is not used and should delete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should I just create another pull request?
It looks like commit message and code differ |
what about read only test ? |
|
@@ -578,7 +582,8 @@ class ThingDevice { | |||
|
|||
void setProperty(const char *name, const JsonVariant &newValue) { | |||
ThingProperty *property = findProperty(name); | |||
|
|||
if(property->readOnly) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good!
Please lint source and squash |
Hi,
I noticed that read only parameter is not used. If a user put value to a read only property it will be accepted.
Also about integer and number properties if the put property be less than minimum or more than maximum the server wouldn't care and accept the new value for the property.
I made some changes to prevent accept new value in mentioned case.