beagleboard / librobotcontrol

Robotics Focused library for embedded Linux computers. Mirror of https://git.beagleboard.org/beagleboard/librobotcontrol
https://beagleboard.org/librobotcontrol
MIT License
194 stars 154 forks source link

Encoder position precision clarification #182

Open clarkbriggs opened 4 years ago

clarkbriggs commented 4 years ago

Is your feature request related to a problem? Please describe. Not an error, but clarification of precision. I am trying get a common implementation of BBs vs say RPi. Comparison of current sysFS result size vs prior mmap direct register access which was long int. Back then I thought the hardware was 64 bits. Today the library returns int via atoi( ).

Describe the solution you'd like Of course, a return value that captures the hardware size is best. Is it 64 bits or 32 bits? OBTW, refs such as https://en.wikibooks.org/wiki/C_Programming/stdlib.h/atoi gripe about deficiencies in atoi, like not thread safe. Preferred implementation seems to be atol( ). There is also the issue of mixed long int vs unsigned long int. I'm pretty weak on coding for this difference. I take the BB hardware to be unsigned in nature, but with correct underflow and correct max value, it may look like a signed int. The rc_test_encoder_eqep function clearly returns singed ints and backwards motion shows as negative numbers. Even refs like https://www.tutorialspoint.com/c_standard_library/c_function_strtol.htm don't seem to be careful with signed vs unsigned.

Describe alternatives you've considered As I try for a common user level implementation between BBs and RPis, I have chased the Raspbian encoder kernel driver, which seems to be a standard Linux available driver. It is clearly software based and take two definable GPIOs for inputs. Multiple instances are allowed. There is a dtoverlay to declare them at boot time. The community doesn't seem to use these for robotics but for rotary encoders that are used for volume knobs. The typical max value is small. The size is not well specified. At least, I couldn't find it. I ended up using the relative response that simply returns +1 or -1 and doing by own accumulation. This leaves the max value more in my control. Of course, you can tell me to use extra hardware such as the long-promised Raspberry Beret. But we still need some thought on the software interface design.

Additional context While I'm at it and considering RPis, I started thinking about long duration operations for our bots. To date, driving around the lab floor doesn't challenge the 32 bit size, I suppose. I did start driving an R/C car with a BBB around the parking lot. We need to start implementing features that account for roll over. The TI hardware has interrupts for that. It hurts my head trying to think about how to do that correctly. If we don't do it well ahead of time (and well), it will be very difficult to identify as a problem in the field when the bot is on mile 10 of 50, or whatever.

jadonk commented 4 years ago

As of now, the encoders are only storing 32-bits in the driver:

It looks like the userspace also doesn't do anything to handle overflows and just returns a 32-bit count:

Looking at the TRM section 15.4, it seems there is a way to estimate speed without looking at positions, but not a clear way to do position counts past 32-bits without adding an interrupt routine:

So, do you think the need to add a 64-bit read operation in the library and an interrupt routine in either the userspace library or the kernel? I think we can take that on as an enhancement by the August release.

Is 64-bit enough?

jadonk commented 4 years ago

Is this the encoder used on a Raspberry Pi? https://github.com/raspberrypi/linux/blob/rpi-4.19.y/arch/arm/boot/dts/overlays/rotary-encoder-overlay.dts

That would be this driver:

I hope your robot doesn't move too fast, doesn't have a high resolution encoder, is running an RT kernel and doesn't have too much else to do. This driver is enabling threaded interrupts on every rising and falling edge of the encoder! Probably burning up a whole A72 to watch wheels go around in circles:

clarkbriggs commented 4 years ago

Jadonk: re Is that the RPi rotary enoder? Yes. I use two copies on different pins like dtoverlay=rotary-encoder dtparam=pin_a=5 dtparam=pin_b=6 dtparam=relative_axis=1 dtparam=steps-per-period=4 This way the driver does the quadrature logic and provides +1/-1 events on a /dev/input/event fd. The quadrature logic is in rotary_encoder_irq ( ) function. I think I was asking if this driver is in the stock Linux pile if it could be just included in the Blue distro as is. I would try the same implementation on my Blue as I did on the RPi. And see what happens. After while, if we made progress on what a low-click-rate measurement would be worth, we might combine it and not keep it in the distro.

clarkbriggs commented 4 years ago

Jadonk: re TRM eqep encoder. Yes the discussion in 15.4 near page 2513 talks about rate capture. That is very much what I do with the software encoder. Every click interrupt, in addition to adding up clicks for position, I use timespec differences between interrupts to calculate the duration. 1/duration is the (low slow) speed click rate. Very much like their equation 2. Their observation that each approach suffers at one end of the speed range is correct. I can see a dual approach that is speed sensitive, especially if the interrupt rate gets too high. But then if the eQEP can be run in both modes, or one mode or the other in low speed-high speed ranges, then the CPU doesn't see any different load.

clarkbriggs commented 4 years ago

Jadonk: re position count register size. When I first asked this above, I had looked at the TRM and concluded the position register was 32 bits. I still find that in the TRM you point to. Section 15.4.3 EQEP registers says offset 0x0 is QPOSCNT and its 32 bits. I dug back through an older version of a derivative of James' eqep access code from the days of Black and Robotics Cape. (What AM335X chip was that? Was its eQEP different?) He used direct memory access via mmap to get the position count as a long (which I had to check is 64 bits). He uses return (unsigned long)(pwm_map_base[ch-1] + EQEP_OFFSET +QPOSCNT); and all our derived engineering variables are type long int. (He does mix unsigned with signed but ...) That's how I came to think they were 64 bit values (back in the day). I can't rectify the different approaches, but maybe you can. It probably doesn't matter if today's chip is really just 32 bits. Never mind what used to be.

dlech commented 4 years ago

long (which I had to check is 64 bits).

long on 32-bit Linux is 32-bits.

clarkbriggs commented 4 years ago

Well dlech, that would explain a lot. I have lost track of which system is 32 bit and which is 64 bit. I think all my desktop boxes are 64 bit, but I guess all these little machines (Blues and RPis) are still 32 bit. Trying to retroactively read minds, what was the author's intent when he coded "return (unsigned long)(pwm_map_base[ch-1] + EQEP_OFFSET +QPOSCNT);" back in the day?

dlech commented 4 years ago

Trying to retroactively read minds, what was the author's intent when he coded "return (unsigned long)(pwm_map_base[ch-1] + EQEP_OFFSET +QPOSCNT);" back in the day?

Reading the 32-bit QPOSCNT register (eQEP Position Counter Register) of the eQEP module (EQEP_OFFSET) in one of the 3 PWMSSs (pwm_map_base[]) on the AM3558 SoC.

clarkbriggs commented 4 years ago

Oops my bad. I meant why did the author force the cast to long if int was sufficient?