Manual antenna#113
Conversation
| else: # is manual mode | ||
| msg = Float32() | ||
| msg.data = float(self.manual_value) | ||
| self.bearing_pub.publish(msg) | ||
|
|
||
| def publish_bearing(self, bearing): | ||
| msg = Float32() |
There was a problem hiding this comment.
I can't seem to find the either manual antenna nodes as a node or remapping. not sure what kind of msg it sends, but roboclaw's tracking_bearing expects a normalized value. Are you able to instead send a radian val to publish_bearing()
(It's not letting me comment from the right file so this may be on the original file)
There was a problem hiding this comment.
I think that we're doing it from the webUI, which I was hoping would be able to send a radian value.
There was a problem hiding this comment.
After thinking about it for two seconds yeah I think you're right I'll do that instead
There was a problem hiding this comment.
the code is here: https://github.com/CPRT/webUI/blob/main/src/components/panels/AntennaControlPanel.tsx#L40. Will have to be modified to support radians on the webUI side but that's fine.
Connor can correct me if I'm wrong, but I don't believe the gps modules will stop for any reason without a new msg. There is no calibration so wha you've added will work fine. |
|
haha, I'll build this later to appease the formatter. Just added a backup threshold check so the automatic antenna is +90 degrees away from the rover so Eileen doesn't get blasted 🔫 . @MaybeWilli Take a look through and make sure everything looks fine, I'll approve the PR once I've updated the WebUI to work properly with what you added. |
| if distance < 5.0: | ||
| bearing = (bearing + math.pi / 2) % (math.pi * 2) |
There was a problem hiding this comment.
I do not like this offset and range for it being hardcoded. Strict cut offs are not good as if we are right on the line a bouncing gps value with cause it to swing back and forth. We have manual mode we do not need this
|
This needs to get finished up, formatted and into main asap. If you can't get to it by tomorrow night let me know and I will finish it. @MaybeWilli @Landwhich |
|
Is this tested? |
Removed AutoThreshold parameter declaration and assignment.
This does interface with the webui as expected (with the new pr I submitted last night), though I haven't tested this with the antenna actually moving or with rosbag data. Not sure how much Will has tested. I do not believe much has changed in terms of the antenna movement so would be good to use on a testing day where we need to interface with webui more and see how it handles that. I'll tag you on additional web pr if I haven't yet. |
|
|
||
| self.publish_bearing(bearing) | ||
| # if not manual, proceed as usual | ||
| if not self.manual: |
There was a problem hiding this comment.
From a pure teaching perspective:
The code is more readable to flip this check and a early return for the manual code path. Keeps indentation bloat down and reduces the cognitive complexity of the inverted logic ("is not" is usually marginally harder to reason than is)
3 participants
Someone please verify what svin_valid, base_fix, and rover_fix are and if it's ok to only check those values if currently not in manual mode. If they have anything to do with calibration, we should just not switch to manual mode before the antenna is calibrated.