[RFC] Code simplification about bricknil HOT 16 CLOSED

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

from bricknil.

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.

from bricknil.

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

from bricknil.

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!

from bricknil.

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)

from bricknil.

@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?

from bricknil.

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

from bricknil.

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

from bricknil.

@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.

from bricknil.

@janvrany Thanks.

from bricknil.

@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 :-)

from bricknil.

@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?

from bricknil.

@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?

from bricknil.

'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?

from bricknil.

@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.

from bricknil.

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.

from bricknil.