UbiquityRobotics / ubiquity_motor

Package that provides a ROS interface for the motors in UbiquityRobotics robots
BSD 3-Clause "New" or "Revised" License
24 stars 23 forks source link

Add tic interval topics and fix bat volt readings and enhance test_velocity tool #126

Closed mjstn closed 3 years ago

mjstn commented 3 years ago

The main feature of this pull request is to support publication of wheel tic interval values that come from v41 firmware. We receive these values in a new message and once received publish to /left_tic_interval and /right_tic_interval. If these messages never arrive due to older firmware this firmware does no new publications.

A second part of this change is to fix an error in our voltage reporting to get more accurate battery voltage readings. Additionally with rev 5.3 boards they will be even more accurate due to a hardware fix on that board.

A third part of this change is to add to the abilities of a script called test_velocity.py With no options it acts like before but we add several options as follows:

JanezCim commented 3 years ago

Testing: MCB 5.2 RPI4 1.) tested with old firmware on the robot. Nothing is posted on /left_tic_interval and /left_tic_interval as expected

ubuntu@thor ~ $ rostopic echo /diagnostics | grep -A 1  'Firmware [DV]'
        key: "Firmware Version"
        value: "35"
--
        key: "Firmware Date"
        value: "20190815"
^C
ubuntu@thor ~ $ ^C
ubuntu@thor ~ $ ^C
ubuntu@thor ~ $ rostopic list | grep interval 
/left_tic_interval
/right_tic_interval
ubuntu@thor ~ $ rostopic echo /left_tic_interval 
waited for 30 seconds and noting happened....
^Cubuntu@thor ~ $ rostopic echo /right_tic_interval 
waited for 30 seconds and noting happened....

2.) Can you point me to v41 firmware so I can install it and test this further?

mjstn commented 3 years ago

firmware that pushes the tic intervals to this change is now posted in the SLACK firmware channel

JanezCim commented 3 years ago

after installing the v41:

ubuntu@thor ~ $ rostopic echo /diagnostics | grep -A 1  'Firmware [DV]'
        key: "Firmware Version"
        value: "41"
--
        key: "Firmware Date"
        value: "20210405"
^C
ubuntu@thor ~ $ rostopic echo /left_tic_interval
data: 2000
---
data: 2000
---
data: 359

The data did not start comming in until after i started teleoping the robot, /right_tic_interval is also posting.

This is how the /left_tic_interval looks like when i just shortly click the forward button on theleop:

data: 208
---
data: 208
---
data: 208
---
data: 208
---
data: 2000
---
data: 415
---
data: 131
---
data: 131
---
data: 131
---

Not really sure what to make of this. If this was tick rate, it should be 0 when the robot is still, but its 131 or 208. If its tick count, i dont know why 2000 is there.

JanezCim commented 3 years ago

Battery level test: the topic /battery_state is reporting voltage 25.2248401642V, with my voltmeter I measure 24.99V. Any other tests that I should I be doing there?

Should i be testing test_velocity.py, if yes, how?

mjstn commented 3 years ago

I am suppressing zero values so that is expected. My logic is that I cannot measure without a train of changes in the encoder. A value of 0 would mean infinite speed and if the robot is stopped the interval becomes infinite.

I don't have a good solution that would not be 'tricky' other than what I did.

So any directions explaining this feature would say something like 'the robot must be moving for this value to be published'.

JanezCim commented 3 years ago

Yeah, so maybe it would be better then to actually not publish onto the /left_tic_interval at all when the robot is not moving.

mjstn commented 3 years ago

The numbers published are a scaled version of time between tics. When going fast the number is smallest and I think is around 200. I tried to make the numbers be scaled so they fit in a 16 bit unsigned number for 'resonable speeds'. So as you get slower the interval goes up. The value you see at 0.5 M/sec is twice the value of the interval at 1 M/sec and the number at 0.25 M/sec is 4x the interval of when it was 1 M/sec. The number is 'time interval between encoder tics'. It is a divided down microsecond counter. I have to work out the exact scale factor but there is a scale factor that is fixed. The other problem is I put in a max interval so when we go really slow the number still fits in a 16 bit unsigned value but at some point I cap that number to a max. The value is thus ok above some really slow speed. We have this serial link to MCB and it is 'precious' in terms of available bandwidth and we do assorted tricks to limit how much we put on there. So I kept this value to 16 bit per wheel at this time. We send a LOT of stuff already. The number is also always positive so you have to know that we are moving a given way to then interprit this number as an interval between tics but you don't know which way we are moving with just this number.

mjstn commented 3 years ago

I think you should see if any stupid bug is in this that would blow up things and not be overly concerned with precise testing.
I also don't expect you to worry a lot about test_velocity.py it is a lab test tool. I was tempted for example to just push it because of the testing I have done. If it were an official part of our featureset it would be different but it is this obscure tool that is not even documented in any of our docs yet anyway sort of like my 'test_motor_board.py' which is EXTREMELY full featured but nobody EVER has reviewed it ever. It is a lab test tool we don't want to loose but not a part of our working codeset.

mjstn commented 3 years ago

Good catch on README. I have in fact added many published topics some of which were from 5 or more years back. Need to inform people on all these things the motor node publishes and accepts. I also removed the diagnostic cap on the interval that you saw. I had a 2000 in there to do some special evaluation.