Ashell refactoring, part 1

[zolkis]

Cosmetic changes to ashell, preparing the way for part 2 (more intrusive changes).
Each commit tested with ashell functionality (run, stop, save, list, rm, ...).
Suggest merging this before refactoring part 2 (which is in private files now).

[grgustaf]

If you want to preserve the individual commits, then please rebase to make it apply cleanly, removing the last patch. Also please use [ashell] in the commit titles just to follow our practice.

Otherwise, we could just squash it into one commit and fix the title ourselves.

@brianjjones please review at this today if at all possible.

[grgustaf]

The first patch committed the generated Makefile in src/ which shouldn't be done.

[grgustaf]

Weird stray stuff here, and below on 565, but I guess it gets removed later on.

[grgustaf]

Maybe just a good reason for us to go with the 'squash' method :)

[grgustaf]

Note to self: reviewed first 7 patches so far.

[brianjjones]

This all looks good to me. I built it both with 'ide' and 'ashell' and didn't notice any issues.

@grgustaf you should however note that this will break the ability to use the 'run' button on the IDE. You can still connect to the IDE, you can use the console in the IDE, but you can't use 'run' to copy JS to the device. This is because the new code to communicate between the two hasn't landed yet, and the website has to be updated. I can't think of an elegant way to update all these things at once, so I'm fine with this temporary breakage. Regular command line ashell works as expected, so you can still read, write, run JS files etc.

@zolkis can you give us an idea how long the IDE will be unable to copy files to the device? I'd hate to break it for an extended period of time.

Thanks!

[grgustaf]

Yeah, we definitely should minimize that downtime. I'm actually leery of merging it if that doesn't work, is that really required?

[zolkis]

These patches so far should work exactly as ashell before. These are just cosmetic changes so far. I have tested each commit with the Run button and they all worked. So if it breaks, it should be fixed here, but seems to work for me.

It is fine to merge them by squashing.

[zolkis]

Found and (hopefully) fixed a problem when running programs after each other. Please check if it works.

[grgustaf]

I flashed this latest version but it seems the Run button is disabled in the IDE. I assume that's what @brianjjones meant by not being able to run apps, so no it doesn't seem to be fixed. Other comments don't matter but this one we should address; need to keep ashell functional in case it takes a while to get the remaining patch ready.

[grgustaf]

Ah, I hadn't used the IDE for a while... now I realize that after I connect I see "connection established" in the console window, but I never see the prompt come back w/ available commands etc, so it's not really up to the console prompt and that prevents it from running apps. We'll have @brianjjones confirm whether that's exactly what he sees. I'll maybe try git bisecting the patch to track down where the problem was introduced.

[zolkis]

I certainly don't have the Run button disabled, actually never was, and the command list also appeared all the times. The problem I noticed was with multiple runs, of different files, that they silently failed. With the latest patch it behaves the same way as with the commit before this PR. I have tested it on Fedora 26 with Chrome, clicking on the desktop notification for WebUSB.

[zolkis]

I wonder if there is a difference between my local build and yours, e.g. on deps/zephyr. I will try to reproduce the problem - unfortunately I don't have a board with me now.

[brianjjones]

It all works for me now, including the run button. It appears we may have a bug in our master that causes the connect to not work sometimes. But this isn't caused by @zolkis 's code here, its already present in master.

LGTM now.

[grgustaf]

Okay we think we've confirmed that it works as well as it did before. +1