IEEERobotics / bot

Robot code for 2014.
BSD 2-Clause "Simplified" License
18 stars 11 forks source link

Clean up targeting_solver and gunner #107

Closed dfarrell07 closed 10 years ago

dfarrell07 commented 10 years ago

I did a quick hack integration of @PaladinEng's targeting code and gunner, to test it on the bot. That logic needs attention, mostly just cleanup.

napratin commented 10 years ago

I was wondering where the targeting solver is getting the bot's position information from? Doesn't it still rely on a localizer?

The current USLocalizer depends on US sensor work to be completed (see #69 for a recent comment on that). Once we have (x, y) position from the localizer, I'm assuming gunner.calc_pitch and gunner.calc_yaw need to be called to get the pitch and yaw, and those need to be passed to gunner.aim_turret? There's a TODO in Pilot for just that :)

dfarrell07 commented 10 years ago

I'm glad you pointed this out, I now see that it's less clear than I thought. I mentioned in #8 that we don't need a localizer concept, and that I was planning on nuking it. I got distracted putting out fires and such and never looped back around to follow through.

The plan is to call USHub from gunner to get our position, then pass that to the targeting logic (again, still in gunner). The functions gunner.calc_* are what I quickly hacked in to gunner so @PaladinEng and I could do an integration test. They work, but they are a quick hack. We'll want to figure out a cleaner way to integrate the targeting logic and gunner.

dfarrell07 commented 10 years ago

Nuked localizer in 7eaaaaf5b3206136f2ddcee29988eff1e99fa471.

napratin commented 10 years ago

Ah, okay. But USHub doesn't return position, only raw US distances. USLocalizer was a lightweight wrapper on top of USHub to return an (x, y) position that gunner's functions could use. If you haven't implemented the US sensor -> (x, y) computation, I'd recommend reinstating USLocalizer or pulling it from there (I agree that Localizer and USLocalizer were probably stubbed out to do lot more than required, and could be trimmed down if needed).

dfarrell07 commented 10 years ago

USLocalizer was a lightweight wrapper on top of USHub to return an (x, y) position that gunner's functions could use. If you haven't implemented the US sensor -> (x, y) computation...

What kind of computation are you talking about? The targeting script takes in a distance from the left edge and a distance from the front, which should require no more computation than pulling that data from the dict returned by USHub. We'll need to add a constant distance to account for the difference between the position of the US sensors and the gun (need to define in config), but that's all I can think of. I guess we could break that out into Localizer, but it seems pretty specific to targeting and firing, which is why I thought it could cleanly live in Gunner.

PaladinEng commented 10 years ago

Where is targetingSolver supposed to live? I thought you had moved it from bot/scripts/ but I still see it there. I need to update the initial launch velocity. any chance we're going to get to test fire soon? Like this evening perhaps?

On Tue, Mar 11, 2014 at 3:59 AM, Daniel Farrell notifications@github.comwrote:

USLocalizer was a lightweight wrapper on top of USHub to return an (x, y) position that gunner's functions could use. If you haven't implemented the US sensor -> (x, y) computation...

What kind of computation are you talking about? The targeting script takes in a distance from the left edge and a distance from the front, which should require no more computation than pulling out of the dict returned by USHub. We'll need to add a constant distance to account for the difference between the position of the US sensors and the gun (need to define in config), but that's all I can think of. I guess we could break that out into Localizer, but it seems pretty specific to targeting and firing, which is why I thought it could cleanly live in Gunner.

Reply to this email directly or view it on GitHubhttps://github.com/NCSUhardware/bot/issues/107#issuecomment-37271014 .

dfarrell07 commented 10 years ago

Where is targetingSolver supposed to live? I thought you had moved it from bot/scripts/ but I still see it there.

Your targeting script is gunner/targeting_solver.py. There was an old version in scripts/. I just removed it to prevent confusion.

any chance we're going to get to test fire soon? Like this evening perhaps?

Obviously the goal is to do that ASAP. But yeah, I think tonight is realistic. Need to closed this issue and #106.

PaladinEng commented 10 years ago

I'll plan on coming tonight.

Sent from my iPhone

On Mar 11, 2014, at 3:36 PM, Daniel Farrell notifications@github.com wrote:

Where is targetingSolver supposed to live? I thought you had moved it from bot/scripts/ but I still see it there.

Your targeting script is gunner/targeting_solver.py. There was an old version in scripts/. I just removed it to prevent confusion.

any chance we're going to get to test fire soon? Like this evening perhaps?

Obviously the goal is to do that ASAP. But yeah, I think tonight is realistic. Need to closed this issue and #106.

— Reply to this email directly or view it on GitHub.

jschornick commented 10 years ago

@PaladinEng -- great! I'll try to work with you to integrate the new ultrasonic data and get it passed into your targeting logic. If we need to make some adjustments to how the Ultrasonic class presents things, tonight is the night to get that figured out.

napratin commented 10 years ago

The targeting script takes in a distance from the left edge and a distance from the front, which should require no more computation than pulling that data from the dict returned by USHub.

@dfarrell07 Yes, that's true - this computation I speak of is no rocket science! And I don't know if targeting_solver is already taking this into account, but you should keep in mind that the US sensors measure distance from the edge of the bot, whereas targeting I believe needs the bot center location (USLocalizer was designed to read these offsets defined in config).

Plus, if follower has to turn in order to reach the blue blocks (and remains turned), then left/right and front/back sensors would've been rotated.

jschornick commented 10 years ago

The logic to fully integrate localization, targeting, and position has been streamlined and refactored into Gunner per 06797d8d79967870108783689949d59e02172eb8.

Regarding localization logic, Gunner calls its own localize method, passing it the results of reading the ultrasonics. This localization is current delegated to a dumb_localize method that simply returns an (X, Y, theta) assuming the bot is aligned square facing the target. This assumption is invalid when we are "centered" on the arc. Further localization can be tracked in #121.

The full integration between targeting and Gunner still needs to be verified/debugged via an integration test on the course.

napratin commented 10 years ago

Sounds good. I just looked at the code, and had one comment: I believe gunner.fire() should be kept lightweight, so that we can fire multiple darts when testing without having to go through localization and aiming steps. These pre-fire steps can be separated out to an aim() method (or even re-purposed aim_turret()) that takes no parameters and calls localize() (later it could be made smarter to sweep horizontally if CV doesn't see the target at first try).

Pilot expects this behavior as well, i.e. two steps aim and fire, mainly because it needs to optimize the time spent at each block - we want to stay at least 3 secs (as per rules), but not much longer. The way I plan to implement this is to time the aim call, and wait any additional duration as required (there's no point waiting after firing, which should be almost instantaneous).

jschornick commented 10 years ago

@napratin -- I agree. The first refactor was mainly done to squash the three layers of Gunner, WheelGunner, and WheelGun into something simpler while converting over to the DMCC motor interface.

I'm going to take a quick stab at pulling the aiming logic out of Gunner.fire() and into Gunner.aim() so that we can call the two pieces separately. The aim_turret(x,y) method will probably go away in its current state, since it's only acting as a unnecessary wrapper around adjusting each axis.

We have been using Gunner.gun.fire(), btw, to do repeated shots in the past, and the new Gunner.fire() will likely be a near-empty wrapper around this call.

jschornick commented 10 years ago

Some additional rework of Gunner's methods was done in commit 0236510d1d4106a43bbe90e628ea61ebbda850e4

jschornick commented 10 years ago

I took some time to verify the velocity values produced by the DMCCs, comparing them to a velocity calculated from delta_position_ticks/period. They are clearly not the same, and with @dfarrell07's help we confirmed that the scaling is linear.

I've added this scaling factor to WheelGunner for now (though it probably belongs in the DMCC library). The important good news is that WheelGunner.dart_velocity now reports sane values that can be fed directly into the targeting solver. The validity of this value will have to checked in the upcoming integration tests.

See: b6afcae3ccfcf89109028ec5389d49836a868bcc

dfarrell07 commented 10 years ago

Jeff cleaned this up.