microsoft / jacdac

Device and service catalogs for Jacdac.
https://aka.ms/jacdac
Creative Commons Attribution 4.0 International
66 stars 25 forks source link

semantics of button service #289

Closed tballmsft closed 3 years ago

tballmsft commented 3 years ago

What is the meaning of long_click and hold? Here is the current service spec: https://microsoft.github.io/jacdac-docs/services/button

tballmsft commented 3 years ago

@mmoskal said:

[10:57 AM] Michal Moskal
    so hold and long_click are the same but with different timeouts
    long_click used to be emitted with up
    but that makes for a really bad ux
    you hold it, and you don't know if you held it long enough
    we should rename: long_click -> hold, hold -> long_hold
jamesadevine commented 3 years ago

We have used the following button semantics from the inception of MakeCode/micro:bit dal/CODAL:

Click: down, delay < 500 ms, up, click. Long click: down, delay of 500 ms-1500 ms, up, long click. Hold: down, delay >1500ms, hold, up. There may be an indefinite time between hold and up.

mmoskal commented 3 years ago

Well, this doesn't mean it's a good idea. I've never seen an appliance (even such wonder of UX design as my dishwasher) that would do something on long click in the above definition - the reason being there's no feedback - you hold it, and hold it, and you never know.

mmoskal commented 3 years ago

another way to slice it, is:

Click: down, delay < 1000 ms, up, click. Hold: down, delay >1000ms, hold, up. There may be an indefinite time between hold and up.

pelikhan commented 3 years ago

We should drop long click and keep hold.


From: Michał Moskal @.> Sent: Wednesday, March 17, 2021 7:30:25 PM To: microsoft/jacdac @.> Cc: Peli de Halleux @.>; Assign @.> Subject: Re: [microsoft/jacdac] semantics of button service (#289)

Well, this doesn't mean it's a good idea. I've never seen an appliance (even such wonder of UX design as my dishwasher) that would do something on long click in the above definition - the reason being there's no feedback - you hold it, and hold it, and you never know.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Fjacdac%2Fissues%2F289%23issuecomment-801314971&data=04%7C01%7Cjhalleux%40microsoft.com%7C01ee315d94bc49c0f5bf08d8e972bb62%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637516026275220892%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=MyIQreydHng%2FEQ8QyVcabJlrv77EhiecL1UkjwnbwMU%3D&reserved=0, or unsubscribehttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAA73QKM2ILUAUB7M4HJW3ILTEDYMDANCNFSM4ZLC3YHA&data=04%7C01%7Cjhalleux%40microsoft.com%7C01ee315d94bc49c0f5bf08d8e972bb62%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637516026275220892%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=i80%2BD7GDQgs%2F2g3vx1h%2FGddmukhfemUOFaom1clrDZY%3D&reserved=0.

jamesadevine commented 3 years ago

If we are dropping an event, I'd like to retain the ability to roughly know how long a click or a hold is. Having differentiation is important when considering accessibility scenarios. We should also talk about the ability to tune the length of time it takes to trigger a click or a hold.

mmoskal commented 3 years ago

The up event could have a time argument. If you want differentiation you could then implement it that way (just ignore click events).

Or we could add click_threshold register, but that may be overdoing it ?

tballmsft commented 3 years ago

We have flexibility with MCU, so I think adding some time information is a great idea.

pelikhan commented 3 years ago

We never used long click in MakeCode btw.


From: Tom Ball @.> Sent: Wednesday, March 17, 2021 10:30:49 PM To: microsoft/jacdac @.> Cc: Peli de Halleux @.>; Assign @.> Subject: Re: [microsoft/jacdac] semantics of button service (#289)

We have flexibility with MCU, so I think adding some time information is a great idea.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Fjacdac%2Fissues%2F289%23issuecomment-801456213&data=04%7C01%7Cjhalleux%40microsoft.com%7Cdc3309540b1946e7c8b108d8e98bef21%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637516134503921479%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=jNzSs1iO3Fx7s3P0pQvlZ0NBYYLXWAHLl9w%2BNgYFbnI%3D&reserved=0, or unsubscribehttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAA73QKLXSOXVXQVJEEJOBQ3TEENQTANCNFSM4ZLC3YHA&data=04%7C01%7Cjhalleux%40microsoft.com%7Cdc3309540b1946e7c8b108d8e98bef21%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637516134503931473%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=WIiO8IKendZOxStFLjTaZpCcTpkT%2BsH2Frgoec%2F09vk%3D&reserved=0.

tballmsft commented 3 years ago

Do we need some design principles around services? Like exposing time for time-sensitive actions/events.

mmoskal commented 3 years ago

I think we'll learn more as we try to use these services.

So I propose: removing long_click, changing click/hold threshold to 1s, and adding time to up event. Anyone against?

jamesadevine commented 3 years ago

I think that sounds good. Time might be hard on the padauk? Maybe it'll be fine. At least we will be able to tune the event detection in software.

mmoskal commented 3 years ago

well, whatever we do, I do think we'll need at least 24bit timer on Padauk

tballmsft commented 3 years ago

OK. I will make changes to spec and the C code.

tballmsft commented 3 years ago

I was thinking to add a register to allow programming of the hold time (minimum 500ms)

pelikhan commented 3 years ago

We need to be careful about setting ranges so that we don't end up with franken-buttons that all behave differently.

tballmsft commented 3 years ago

OK. I was thinking that the default would be 500ms. Do you prefer a small set of presets? Or just jettison the idea altogether? We would have tests to ensure a common default.

pelikhan commented 3 years ago

What are the scenarios that would require to change the timings on the buttons?

pelikhan commented 3 years ago

Let's track this here: https://github.com/microsoft/jacdac/pull/292

jamesadevine commented 3 years ago

If someone is not able to press a button in the time frame we define. For example, if someone has cerebral palsy, it's unlikely they will be able to press and release a button in under 500 ms. They will never be able to generate a click event.

pelikhan commented 3 years ago

Okie. So we have:

mmoskal commented 3 years ago

I think for sanity we should have click min time == hold time.

pelikhan commented 3 years ago

For many bike lights, it's not uncommon to use "hold" as a way to turn off the device. That hold requires several seconds to trigger, which click is just a few hundred ms.

tballmsft commented 3 years ago

I think for sanity we should have click min time == hold time.

Ah, a dependent type! But seriously, is this just English in spec with some tests?

mmoskal commented 3 years ago

I think we should just have one register, and in the English description we use it for both events.

tballmsft commented 3 years ago

I don't understand how you can use one value for both events. click is faster than hold. I guess I don't understand what you mean by click min time == hold time.

mmoskal commented 3 years ago

I think there should be one threshold. If the button is pressed less than that, you get a click, and if it's pressed more than that you get a hold.

tballmsft commented 3 years ago

OK. I'm not sure that will work for James' accessibility requirements... well, actually maybe it does!

tballmsft commented 3 years ago

I updated the PR.

jamesadevine commented 3 years ago

I think the threshold would work.

I'm just worried about arbitrarily making decisions without going and asking the people who use the hardware. What if in a few months we decide these button changes were net bad? Sure we can change the spec, but if we're going to be burning button code into OTP flash (i.e. on PADAUK) then we have no facility to change button behaviour in the future and no backwards compat story.

mmoskal commented 3 years ago

It's a button, how complicated it can be? :) If we put that much deliberation into other services we would never get anywhere.

Anyway, you can implement whatever behavior you want with the timestamp in up event.

jamesadevine commented 3 years ago

Have a look at other software implementations and see for yourself how much thought goes into button API design :smile:

mmoskal commented 3 years ago

I think I value my sanity too much...

;)

pelikhan commented 3 years ago

Is this done? @tballmsft

pelikhan commented 3 years ago

I got some new headphones and they use 2 second = on, 3 seconds = off

mmoskal commented 3 years ago

Let's do what @pelikhan suggested - let's send hold event with duration every 500ms. Let's also remove the click event and click_threshold - this will simplify impl on Padauk - we'll never (...well, very rarely) have to send two events right after another - and it will also simplify the mental model - it's up to the client to decide what is hold and what is click.

mmoskal commented 3 years ago

(which is not to say we should encourage people to design such horrible interfaces, like two different functions on 2s and 3s hold...)

tballmsft commented 3 years ago

Interesting idea on repeated hold event - I am happy to implement/test. But it's strange to me to not have a click event for clicks < 500ms. OK with you @jamesadevine?

jamesadevine commented 3 years ago

So we are proposing to remove the smarts from the button? What is the microcontroller actually doing in this case then?

mmoskal commented 3 years ago

Well, if we send the time the button was down, this is strictly more information than a click event. Similar for repeated hold.

My general rule of thumb is that programmable logic belongs on brain, unless there are compelling reasons to put it on the module (eg. because of limits in the transmission rate, or because the logic depends on hardware (eg. servo gets degrees not microseconds because that can be calibrated by the manufacturer).

tballmsft commented 3 years ago

On the other side, we need to have the module expose enough information for the brain to make decisions. We have added time to up event and are now repeating the hold event every 500 ms and eliminating click, so I think the brain has all the information it needs with up, down and hold (though we are fixing the hold to only send every 500ms). Everyone OK with this?

tballmsft commented 3 years ago

Unless I hear otherwise, I'll work on this tomorrow.

tballmsft commented 3 years ago

Here is the updated spec and tests: https://github.com/microsoft/jacdac/pull/315 and implementation: https://github.com/microsoft/jacdac-c/pull/19

tballmsft commented 3 years ago

done

tballmsft commented 3 years ago

Reopening this issue. To recap, we have now (implemented in spec, firmware, test):

The only issue here I see is that we force the user program (client) to recognize sequence of events in order to implement click and long-click. We could have a concept of client-side events that are functions of traces of server-events.

pelikhan commented 3 years ago

Or have "hold", "long hold", no repeats.

mmoskal commented 3 years ago

You don’t need traces to detect events on client (you may want to check them for testing though) - up with time less than 300ms or whatever is click and hold with time between 500 and 999 is hold (where the client sets the thresholds)

tballmsft commented 3 years ago

Good point, Michal. There's still some logic needed. My point is we could define such events and have code generated automatically.

tballmsft commented 3 years ago

I think we should keep the repeated hold event. It's useful to have the MCU communicating that the button is still being pressed (like a "i'm still alive" at the user-level). Otherwise, we are dependent on client code getting the up event, which it might miss.

mmoskal commented 3 years ago

Yes, definitely keep them - they allow client code to respond to any hold period (with 500ms granularity)

we could have synthetics events; would it also require synthetic register (click time, hold time)?