Skip to content
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

iOS Proximity Sensor #362

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

sumitmadhwani
Copy link
Contributor

No description provided.

else:
return str(False)
else:
return 'Proximity Sensor in present in your device.'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"is not present" ?

def _get_proximity(self):
if self.device.proximityMonitoringEnabled:
if self.device.proximityState:
return str(True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why now returning a string of a boolean, both platforms are coded to return a boolean value anyway?

@@ -37,7 +37,7 @@
Label:
text: 'Does Proximity Sensor detect something?'
Label:
text: 'Yes' if root.is_near else 'No'
text: root.is_near
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tshirtman I was trying to remove this condition statement from here. That's why String Property.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's a good idea to change the api to simplify a particular example, especially since a lot of other usages would change from:

if obj.is_near:
    do_something()

to

if obj.is_near == 'True':
     do_something()

which is really un-pythonic, the api is better when a boolean value is represented as a boolean.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, you could just change the example to this, to achieve the same effect as your previous solution.

text: str(root.is_near)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"text: str(root.is_near)"
👍

@tshirtman
Copy link
Member

If someone can test it, making the api returns a boolean again is easy, if it works it can be merged after that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants