Standardize commands with BUTTON

[doegox]

trafficstars

I started implementing changes to make the client waiting for the user to press pm3 button rather than immediately returning to prompt because it's strange to get a prompt when pm3 is busy and it hurts scripting (I did all LF sim commands, so you can e.g. proxmark3 -c "lf sim"). But now I realize that in all armsrc BUTTON usages, we have:

• 29 interruptible via USB: while (!BUTTON_PRESS() && !usb_poll_validate_length())
  • including the LF ones while my commit make impossible to send a usb cmd, oops
• 38 solely interruptible by button: while (!BUTTON_PRESS())

We should have a consistent approach for all these commands.

1. We use only BUTTON_PRESS and we make the client waiting for the button to be pressed (with a message to tell the user to stop the command by pressing the button).
2. We allow the command to be also interrupted via USB but I've a few questions on that one:

• is it useful?
• is it intuitive to have to send e.g. "hw ping" to interrupt the running command?
• how to tell it clearly to the user?
• how to use it in batch mode? proxmark3 -c "lf sim" would quit immediately

3. We allow USB interruption but we present it better in the client. E.g. a special prompt to tell a command is running and will be interrupted if pressing enter. So we don't accept other commands and as soon as readline receives sth (so enter got pressed) we send a dummy command to Pm3 to interrupt the running simulation. Or the button is pressed and we get a reply PM3_EOPABORTED and go back to normal prompt.

[doegox]

There are also obvious errors

armsrc/lfops.c:        if (BUTTON_PRESS() && !usb_poll_validate_length()) 
armsrc/mifarecmd.c:                if (BUTTON_PRESS() && !usb_poll_validate_length()) {
armsrc/mifarecmd.c:            if (BUTTON_PRESS() && !usb_poll_validate_length()) break;
armsrc/iclass.c:        if (BUTTON_PRESS() && !usb_poll_validate_length()) goto out;
armsrc/iclass.c:        if (BUTTON_PRESS() && !usb_poll_validate_length()) break;

Should be if (BUTTON_PRESS() || usb_poll_validate_length())

[iceman1001]

I think we talked about this. Part problematic.
Interruptable functions / loops on device side is a must. We didn't have in before, so device could start looping something and only way to stop it, was to disconnect your device.
So BUTTON_PRESS came, however that wasn't always good enough. It is a manual step which demands a person to actually press the button.
So enter "check on usb if a message came" function, which enables us to interrupt device loops (who supports it) on client / standalone, and break out of loops to start doing other stuff. Like break out of sim, try new data, enter sim again.

Idea was also so check is package arrived over FPC..

However problems still are there. BUTTONPRESS & usb_poll_validate, takes time to execute. In time sensitive loops this is not possible to do. As a solution, we started recently to add an extra counter, so in a timesensitive loop (like sim, lf aquiredata) we only check BUTTONPRESS/usbpoll every 1000 times in the loop. That seems to give a ok user experince with pushing button an somehting is still happening / and the timing sensitive loop still succeed in its purpose.

Some of the loops, the BUTTONPRESS /usbpoll is negated, which you saw. Before assuming those are errors, verify that it isnt a negated check in the loop logic. ( i know but it isn't the strangest thing I have seen in this code)

All loops that implements BUTTON press only, should also implement usb_poll, and make sure it guarded with 1000th time execution.

For combining of commands on client side / standalone, we must have a way to tell the loops on device side to break out.

It is a good suggestion to be more clearer to the user that device is now going into looping and need extra steps. Wise from experience , ppl doesn't understand that either. Even a two line message isnt clear enough. For coding purposes, sending a new usb command and with it break the current loop is a must. however, it has shown that you can't send any other command. since that message will be lost on the device side ( I know, strange) , so sending a ping, was good before, now ping changed.

Keeping track of which state the device is in (like a loop for sim) would be interesting and send a "break and return to normal operation" command but I don't see it as a feasible implementation.

The device is a rough by nature. We could have a nasty global variable on device, that we must set before and after one of these "breakable" loops.