Feature: Add Pinch (in/out) support

[pixel-shock]

@sds100 As I said, I added Pinch support

What I did:

• add pinch support
• fix max finger count on swipe and pinch gestures (seems so be a bug in Android that you're not able to use 10 fingers, because in the code of the GestureDescription.java:176 the devs wrote "if (mStrokes.size() >= MAX_STROKE_COUNT)" and "MAX_STROKE_COUNT" is 10 so 9 fingers are the maximum currently (added this to the label of the input fields and to the validator for the save action button enabled)
• change icons for swipe and pinch gestures
• solved some code format issues (redundant semicolons etc)

[sds100]

I'll take a look at this PR later this week. I don't understand the issue with the max 10 fingers bug. Doesn't if (mStrokes.size() >= MAX_STROKE_COUNT) allow 10 fingers?

[pixel-shock]

No hurries 😉

No, the ">=" includes 10, which is really strange because normally everyone should have max 10 fingers. If I allow 10 fingers in the input fields then the GestureBuilder will throw an Error with "too many strokes" because of that line ...

The line if (mStrokes.size() >= MAX_STROKE_COUNT) is the if-clause for the exception, so if the strokes are more than 9, it will throw the error ... really strange in my opinion

[sds100]

Ah, I see now. Thats the catch for the exception. Yeah, thats strange.

[pixel-shock]

I've added a check for StrokeDuration and "finger"Count with a proper message ... and reverted my last 2 commits regarding a min sdk version bump, so if the user enters a duration which is too high or setup a finger count which is too high, then it will trigger a proper error message

[sds100]

Hi @pixel-shock, can you also document this feature in the actions.md file in the docs folder? We have a website documenting features in the app. Please put any limitations of the action there as well.

[pixel-shock]

Hi,

sure, I'll do it tomorrow 👍😉 or today in the evening.

[pixel-shock]

@sds100 Added the docs, also for the swipe gesture. If I missed something please let me know 👍

[sds100]

Can you explain what Radius is in the UI? Maybe suggest good values. It took me a while to figure it out. It is how far away from the "end" coordinates the fingers should be right?

[pixel-shock]

Hi,

jep exactly. Maybe it should be also "distance" to be more clear? Maybe radius is a little bit to much math-based?

[sds100]

yeah, "pinch distance" or something?

[pixel-shock]

I was to quick, I've just added more explanation to the docs, but I'll change the "radius" to "Pinch distance", makes more sense 👍

[pixel-shock]

Changed it to "Pinch distance" and changed/added the docs in the actions markdown file 😉

[pixel-shock]

Morning @sds100,

I changed "radius" to "distance" in code too, so the code will match the UI to avoid confusions. And as we're talking about "distance" I also changed the param in the "distributePointsOnCircle" function to be divided by 2 because otherwise the distance would be like 200 but as the function uses a radius to distribute the points the gesture size would be 400 then, you know what I mean?

[sds100]

Yeah, that makes sense. I'll look over it again today. Thanks for the work!

[pixel-shock]

No worries, I am happy to get a review on that one because I have a lot to learn and as I am working on the next feature for interacting with elements on the screen it will help me a lot 😉

[pixel-shock]

thanks for the review Seth, really appreciate that 👍

[sds100]

Also, in the future can you fork from develop instead of master. I've set develop as the default branch now so you shouldn't need to do it manually :)

[pixel-shock]

Superb. Thank you very much 🍻

[pixel-shock]

Hmmm ... but I think the current merge conflict is caused by the diverged deleop/master right? The Swipe Screen Gesture is missing in develop currently

[sds100]

Oh right, I'll fix that merge conflict. Also, can I set 200ms as a good default for the duration? I'm also working on showing an error if the finger count is less than 2.

[sds100]

is a duration of 0 allowed or not?

[pixel-shock]

Sure, duration can have a default of 200ms ... 0 should not allowed. I see my fault in the "isOptionsValid" function, sorry for that ... it should be duration > 0

For the fingercount: It should not be possible to enter less than 2 fingers

[sds100]

I'm fixing those issues right now pinching. I'll let you do the work of copying what I did for the old swipe action as well. I would like more error messages there as well.

[pixel-shock]

alright 👍

[sds100]

10 is the maximum allowed finger count?

[pixel-shock]

depends on the Android Version, on Nougat it is 9 (see our discussion about that), that's why I did the check in the accessiblity service, I think they fixed that on newer releases

[pixel-shock]

if (fingerCount >= GestureDescription.getMaxStrokeCount()) {
    return Error.GestureStrokeCountTooHigh
}
if (duration >= GestureDescription.getMaxGestureDuration()) {
    return Error.GestureDurationTooHigh
}

[sds100]

yup, just trying to create an error message for it. it is specific to SDK 24 the issue with it being 9? or also previous SDK versions?

[pixel-shock]

Did not checked that to be honest. I would suggest to add an error like:

The entered Number does not match the current Android limitation of X

That would be very generic

[sds100]

I'll just say its 10. does the app crash on Nougat if you try to use 10?

[sds100]

I just checked and Nougat is 0.7% of active users currently 😄

[pixel-shock]

No it will just throw an error via Toast Notification

[sds100]

I also noticed that you didn't put the functions inside MathUtils inside a MathUtils object. I don't like having functions in the global namespace because its unclear where a functions comes from. I've fixed this but just so you know for next time 🙂

[pixel-shock]

Good point. Thank you very much! 🍻

As I have a cold I will have a look into everything tomorrow. For today I just need some sleep 😬🤷‍♂️

[sds100]

Oh no! Hopefully it doesn't last long 😄 I'll try to be complete with my review tonight

[pixel-shock]

Thx. I Hope that too, I am going into vacation next weekend. Tuesday goLive with a project at work so lot to do 🤪

Awesome. Thx again 🙌 I will try to fix everything then tomorrow 👍

[pixel-shock]

fyi: I could not resist to add androids maxstrokes to the fingercount error - sorry 🤣

[DagJenssen]

I am sorry if I ask in the wrong place, but since the thread is open for comments I will take my chances: I am setting up Key Mapper to allow a six button bluetooth remote controller which sends keyboard strokes to the Android device to zoom, pan and go back to position in Google Maps. It works very well with the swipe feature, but pinch seems to only pinch one way. Whether I choose pinch out or in, it will zoom out. Is this a bug or have I misunderstood the functionality? Coordinates entered are in the middle of the screen and pinch distance is the default, 200px in 200ms. I would be very happy for some assistance on a great app. And if I ask this in the wrong place, please let me know. Dag

[pixel-shock]

@DagJenssen pls open a bug ticket for that one and before doing this please try to increase the distance ... I've encountered that some devices need a higher distance to do a proper action