Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add new motor class for tracking angles without queuing or threading #196

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

iwanimsand
Copy link
Contributor

@iwanimsand iwanimsand commented Mar 15, 2023

I implemented an idea how the use case in #195 could be realized. This is only a draft with missing documentation and I don't know if I used all the things correct! (And my english should be improved. 🙈 )

I created a special motor class which directly writes to the HAT without queuing or threading.

It would be nice if we could discuss the idea and if this would be a way to go or not. Maybe there is an easier and better way without this much code duplication (much code is the same as in Motor class). But I found this the easier way than to add the functionality to the existing Motor class.

@iwanimsand iwanimsand force-pushed the tracktarget branch 3 times, most recently from 2896324 to b85c64d Compare March 15, 2023 20:22
@chrisruk
Copy link
Contributor

Thanks for your pull request, I've just been having a look. I think you're right in thinking you could probably reduce code by either modifying the Motor class, or subclassing it.

I could get the test case you created to pass, by modifying _run_positional_ramp to add a usefuture parameter, which if set to False, behaves similar to your code (although it's setting the PID too, which could be changed) -

    def _run_positional_ramp(self, pos, newpos, speed, usefuture=True):
        """Ramp motor

        :param pos: Current motor position in decimal rotations (from preset position)
        :param newpos: New motor postion in decimal rotations (from preset position)
        :param speed: -100 to 100
        """
        # Collapse speed range to -5 to 5
        speed *= 0.05
        dur = abs((newpos - pos) / speed)
        cmd = (f"port {self.port}; select 0 ; selrate {self._interval}; "
               f"pid {self.port} 0 1 s4 0.0027777778 0 5 0 .1 3; "
               f"set ramp {pos} {newpos} {dur} 0\r")
        if usefuture:
            ftr = Future()
            self._hat.rampftr[self.port].append(ftr)
            self._write(cmd)
            ftr.result()
            if self._release:
                time.sleep(0.2)
                self.coast()
        else:
            self._write(cmd)

And also doing the following in serinterface.py

                elif cmp(msg, BuildHAT.RAMPDONE):
                    try:
                        ftr = self.rampftr[portid].pop()
                        ftr.set_result(True)
                    except IndexError:
                        pass
                elif cmp(msg, BuildHAT.PULSEDONE):
                    try:
                        ftr = self.pulseftr[portid].pop()
                        ftr.set_result(True)
                    except IndexError:
                        pass

I did wonder if there's any downsides to sending commands to the build HAT, but not waiting for the 'ramp/pulse done' message, for example if it could overload the HAT, but maybe not. Will ask the firmware engineer too about that.

@iwanimsand
Copy link
Contributor Author

iwanimsand commented Mar 17, 2023

I could get the test case you created to pass, by modifying _run_positional_ramp to add a usefuture parameter, which if set to False, behaves similar to your code (although it's setting the PID too, which could be changed) -

Thanks for the code, I will also implement your suggestion and test it, it would be much cleaner and fit's better in!

My thoughts were writing the full command

cmd = (f"port {self.port}; select 0 ; selrate {self._interval}; "
               f"pid {self.port} 0 1 s4 0.0027777778 0 5 0 .1 3; "
               f"set ramp {pos} {newpos} {dur} 0\r")

everytime would be slower than setting up the port once and then writing only

cmd = f"port {self.port}; set {1 / 360 * degrees}\r"

to the HAT a new angle comes in. This would minimize the communication over the serial port when sending a lot of commands. But I don't know if it is really faster or not - I could test it when I have time.

I did wonder if there's any downsides to sending commands to the build HAT, but not waiting for the 'ramp/pulse done' message, for example if it could overload the HAT, but maybe not. Will ask the firmware engineer too about that.

That sounds very interesting, I never thought about it when I created a show case vehicle named Ludwig last year which used a trained CNN to follow a line with a camera. The CNN predicted the angle out of the picture and sent the command directly to the Build Hat. This happened 15-20 times per second and I used my own serial interface implementation because I needed to send as much predictions to the steering motor I could (and the feature to overwrite commands for fast steering changes). The code used the same concept you see in the commit of the actual pull request (365e8da - writing directly to the Build HAT without queuing or waiting for 'ramp/pulse done' message). Thanks to the perfectly documented "Build HAT Serial Protocol" (https://datasheets.raspberrypi.com/build-hat/build-hat-serial-protocol.pdf) it was not much work to find out.

The vehicle drove 6 to 8 hours per day for 3 days in a row without any problems.

I would be very interested to hear if not waiting for the 'ramp/pulse done' message could overload or destroy the HAT 😮 - but as I said above: no problems in that 3 days and the HAT still works perfectly today.

I attached some pictures of the vehicle, its inside and the bottom:

ludwig

ludwig_inside

ludwig_bottom

There is a video (https://www.puzzle.ch/wp-content/uploads/2022/06/videoittrans.mp4) embedded in the blog https://www.puzzle.ch/de/blog/articles/2022/06/09/mobilitaet-wird-multimodal-und-datenbasiert (written by my workmate) where you can see it driving. 😄

@chrisruk
Copy link
Contributor

chrisruk commented Apr 11, 2023

Sorry for the delay in reply, I've contacted the firmware engineer who says
No, there's not really any chance of damage.

You're also correct in thinking not setting up the PID each time would be better.

I just watched the video of your little car, that's very cool! I'd not heard of CNNs being used for predicting
an angle of a line before.

The vehicle drove 6 to 8 hours per day for 3 days in a row without any problems. - That's also very impressive!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants