gobot
gobot copied to clipboard
hybridgroup
Robot.AutoRun feature and Robot.done chan
Hello, I think there is a bad implementation out of a proper intention, around robot.done chan and AutoRun feature.
First question I had was around this line : https://github.com/hybridgroup/gobot/blob/dev/robot.go#L196 I like using channels for the idea of notifying a work is done, but to me this is a case where channel needs to be closed. So anybody waiting on this can exit, not only one guy.
Further inquiries showed me that the only place that I can see where the channel actually read, is right here, https://github.com/hybridgroup/gobot/blob/dev/robot.go#L196 - which makes no apparent sense to me - I think this is a bug, maybe the idea behind is would be instead to notify Work() has returned - notifying AutoRun block, that it can safely exit since work is over. (nothing more is to be done)
So yeah, this train of thoughts lead to the 2 patches here: https://github.com/solar3s/gobot/commits/autorun - let me know if you find it relevant
On a side note, I will be working (if I manage to organize my time correctly) on a small Arduino project for a little while, nothing too fancy but there is data to be collected etc. so yeah I want to be up here with go, it's easier for me, so this project seems the obvious choice. I can see there is work to be done though, are there concrete directions to contribute, a roadmap or something you guys want help on?
This kind of stuff I like, channels sync and race funniness.. Not an expert at all but I've been doing a fair amount of it last year on bittorrent stuff in go (see taipei-torrent which was our base fork)
Cheers, Romain
Hi @rkjdid thanks for your thoughts.
The idea, perhaps not well expressed, is that in most cases, what you want is for your Gobot code to run indefinately, letting the various goroutines that have been started up by On() and Once() event handlers run awaiting their events. We called this Autorun == true although in retrospect that was not a good naming. Perhaps it would be better to call it TrapInterrupts? In any case, this is supposed to run until the user interrrupts the program with a Ctrl-C.
In the case of a program that is supposed to just do some Work, and exit afterwards, the condition is supposed to be Autorun == false. Since this is not my own personal usual use case, it has not received the kind of attentions the first case has.
Regarding the implementation of Work being done, etc. it would be much better to switch to using context, since we have already stated the intentions of only supporting Golang 1.7+ for all active Gobot development. I agree that we need to way for Work itself to notify any calling goroutine that the Work is done. However, in the "Autorun" case, the Work is never done, it just gets started, and runs until interrupted.
I really like this conversation and seeing your ideas. What do you think of the above?
Hey @deadprogram, I wanted to give a proper answer but I've been a swamped mofo these last weeks..
I agree with you, the use-case of no Autorun makes sense, and the trap makes sense too. What troubled me is that Work() routine which has litteraly no way of ever doing any more "work", waiting on a chan receive - taking into account the debugging purpose of seeing your routine running makes sense, but I would argue that it can be achieved better with a simple log.Print("work done"), although that might be opinionated.
What I didn't take into account are the event handlers, thus calling bot.Stop() is probably not the way to go I agree with that.
I would really like diving more into the project, unfortunately I've had to switch to a more handcrafted / simplistic communication protocol, since I had an inconsist uint value using firmata adapter when reading on an analog pin, and since I have very little time on this side project I didn't dive in the debugging of that and opted for the simple approach, for now.
for the record the project lies here: https://github.com/solar3s/goregen - is just starting and is about regeneration of standard alcaline batteries : https://regenbox.org/
all hands welcome :)
Thanks for the response @rkjdid if you could provide any details about any problem you encountered with the Firmata adaptor that would be greatly appreciated.
https://github.com/bugst/go-serial looks like a good package, might switch over to that since it has a few cool features such as enumerating serial ports.
yeah that's why I opted for it, but I have no clue who's doing the best underlying implementation, you might want to use only the port-discover feature and keep whatever serial lib you're using.
Currently I'm trying to get a proper way of keeping touch with the serial connection throughout savage disconnects/reconnects, not trivial at all!
It would be nice if the control flow was manageable or at least inspectable externally, this would allow to coordinate with other channels/contexts.
