[RFC] Code simplification

[janvrany]

This is an RFC rather than a report of specific defect.

I'd like to discuss possible simplification of the code. (i) As of now, the library uses special hacked version of BLEAK library (which I see as maintenance problem). (ii) It looks that recent BLEAK supports also Windows and MacOS (or soon it will), therefore we can remove support for other backends in bricknil and depend solely on BLEAK. (iii) Once we do (i) we can remove dependency on curio and run everything in a single thread using asyncio (which is part of python3) (iv) Once we do (iii), we can get rid of multiple "tasks" and message-passing through queue in bleak_interface.py, removing the file altogether.

I prototyped changes for (i) and (iii) and not only it make things (little) simpler but also my 4x4 off-roader is much more responsive (using the original code the reactions are "slow" on my setup)

@virantha, what do you think? Shall it try to push these changes further?

[janvrany]

Ah, I just noticed (ii) has been done by @dlech in #5.

[janvrany]

I gave it a go and and implemented (iii) and (iv) on top if @dlech's PR #5. You may find the code here: https://github.com/janvrany/bricknil/tree/simplify Tested with BLEAK commit 472b576ef76f88a2830c823ed0970b79a032e5a6.

The code is not good enough to make a PR, however any comments would be appreciated.

[dlech]

Maybe you should make a PR anyway so that it is easier to comment?

[janvrany]

Right, that make sense. Let me at least do a quick pass over comments and function names to remove (now obsolete) references to curio. Thanks!

[positron96]

Just to make sure: will this proposal make possible such code: hub1.motor1.set_speed(100) sleep(1) hub2.motor2.set_speed(100)

i.e. not have a separate run method for each hub that needs heavy intercommunication? (the code above is simplified, as I understand, there would be still await, async)

[janvrany]

@positron96: I'm afraid it would not and AFAIK it is not possible now anyways.

However, I agree this is a limitation as one hub might not be enough for more creative models. I'd like separate the concept of a model (and it's run() method) and the hubs themselves. One would then define its own model with one run() and attach hubs to the model. So you can write something like:

class CoolButComplexModel(bricknil.Model):
    def __init__(...):
        ...

   async def run(self):
       self.hub1.motor1.set_speed(100)
       sleep(1)
       self.hub2.motor2.set_speed(100)

While I think this is desirable, such change is outside the (proposed) scope of this issue. Maybe open another?

[janvrany]

@dlech , @virantha : to start a more detailed discussion, I've opened a PR with mu current working code: see PR #11 Code simplification

[justxi]

@janvrany Hi, are all your changes which are mentioned in the PR also in the branch "simplify" in your forked repository?

[janvrany]

@justxi I'll check later today. As this repository seems to be..."quiet" so to say, I went ahead and did more radical refactorings / cleanups (to suit my needs) I'll have a look and push my latest code. Will let you know.

[justxi]

@janvrany Thanks.

[janvrany]

@justxi

Hi, are all your changes which are mentioned in the PR also in the branch "simplify" in your forked repository?

Yes.

Moreover, my current development version is in branch devel - https://github.com/janvrany/bricknil/tree/devel. Let me know if you have more questions / issues (maybe by opening an issue on my repo, not sure @virantha wants this repo be polluted by issues for some other code :-)

[esklarski]

@janvrany I'm just stumbling on this now and wondering what you suggest as a starting point to test your changes? Should I test the 'simple' branch or your 'devel' one?

For now I am trying the devel branch, but I'm also wondering if the documentation of methods has been updated?

[janvrany]

@esklarski :

but I'm also wondering if the documentation of methods has been updated?

Sorry, not sure what do you mean. Can you elaborate? Generally, I tried to update comments in the code, the rest (doc, test) almost certainly is rotten (i.e, not up-to-date). I did not bother with that until I get some feedback on changes in the code, but that did not really happen for one reason or another. Seems to me @virantha is either not around anymore or not interested.

@virantha : are you still here? Any comments?

[esklarski]

'I tried to update comments in the code, the rest (doc, test) almost certainly is rotten (i.e, not up-to-date).'

That answers my question, thanks. I have done just enough to get the Duplo train up and running and so far there are no crashes (that aren't my fault...) and it does seem more responsive. Thanks for the efforts.

If it is all right with you I have one issue and I figure I should open it against you repository as it is more up-to-date, or should I put it here?

[janvrany]

@esklarski

I have done just enough to get the Duplo train up and running

Glad to hear that. I was wondering whether it works with anything but (my) technic hub, thanks!

it against you repository as it is more up-to-date, or should I put it here?

I would say against mine, given that there's no response from @virantha for more than half a year now. That's said, I cannot promise I'll jump on it and fix, I'm fairly busy lately. But Iwill do my best and merge PR's (as long as it won't break my technic stuff :-) So if it's okay with you, let's move to my fork.

[esklarski]

Sounds good, I'm new to Python so there's nothing coming fast. Also there's no obligations coming from me, I understand life is busy.