There are a couple of issues with the current REPL and crowlib code that we should address:
REPL: Use of globals/no proper place to store state
Communication internals are implemented in both the REPL and the CLI as well as in crowlib, mainly because we're directly exposing the serial port object (ie we need to do .write(bytes()) or .write(something.encode()).
Where to write output to from crowlib is passed in from the REPL. Ideally the output from crowlib wouldn't care about this and use normal logging which would just bubble up to consumers of crowlib like the REPL that could then show the logged output.
Things like uploads are implemented multiple times (once in crowlib and once more with a bit more logic in the CLI) and their behavior is different
I've been trying to get something working for a bit (for example in this branch https://github.com/simonvanderveldt/druid/tree/cleanup-crowlib where I started with refactoring crowlib) but it's been difficult to keep the changes small, mainly because everything is a bit entangled with eachother. It tends to end up in basically changing everything :grimacing:
I don't really know how to solve that yet, but I would like to suggest to start with adding a class for the REPL. This should contain the REPLs state and hopefully be a decent start for the other changes as well.
This might need a separate issue but putting it here for now:
We might also need to change the serial implementation a bit, especially the reading of the serial port. Unfortunately asyncio's add_reader won't work cross platform (mainly issues on Windows, serial isn't an fd on Windows which means it won't work with SelectorEventLoop and ProactorEventLoop doesn't support add_reader at all. See https://docs.python.org/3/library/asyncio-platforms.html#windows for more detail.
So we'll either need to fall back to using pyserial-asyncio, which also has issues on Windows which for now have been solved by polling (see https://github.com/pyserial/pyserial-asyncio/pull/8) or we could also simply split the serial reading off into a thread.
There are a couple of issues with the current REPL and crowlib code that we should address:
.write(bytes())or.write(something.encode()).I've been trying to get something working for a bit (for example in this branch https://github.com/simonvanderveldt/druid/tree/cleanup-crowlib where I started with refactoring crowlib) but it's been difficult to keep the changes small, mainly because everything is a bit entangled with eachother. It tends to end up in basically changing everything :grimacing: I don't really know how to solve that yet, but I would like to suggest to start with adding a class for the REPL. This should contain the REPLs state and hopefully be a decent start for the other changes as well.
This might need a separate issue but putting it here for now: We might also need to change the serial implementation a bit, especially the reading of the serial port. Unfortunately asyncio's
add_readerwon't work cross platform (mainly issues on Windows, serial isn't an fd on Windows which means it won't work withSelectorEventLoopandProactorEventLoopdoesn't supportadd_readerat all. See https://docs.python.org/3/library/asyncio-platforms.html#windows for more detail. So we'll either need to fall back to usingpyserial-asyncio, which also has issues on Windows which for now have been solved by polling (see https://github.com/pyserial/pyserial-asyncio/pull/8) or we could also simply split the serial reading off into a thread.Also see #38