search

ClemensElflein / open_mower_ros

Other
498 stars 122 forks source link

publish current mowing area to ROS and MQTT #97

Closed 11phc closed 1 month ago

11phc commented 3 months ago

Purpose

The PR aims to publish the current mowing area integer to ROS and also onboard and external MQTT. When used in a frontend such as HA or NodeRed this will be useful firstly to ascertain quickly which area the mower is trying to mow, without looking at a fast-moving log stream or waiting to see where it physically goes! (useful with multiple maps, and when setting areas up for the first time and trying to repeat the same area). It should also make it easy to see if the mower has just skipped an area because e.g. it cant get to it.

Finally it should be useful in the high-level front end implementations of people looking to start mowing a specific area, and cycle through them with ROS calls until it's on the correct mow area.

Where?

A new "current_area" integer parameter is added to the mower_logic/current_state and xbot_monitoring/robot_state ROS topics, and this is also mirrored on the local and external MQTT brokers. In MowingBehavior mode this integer is the mowing area index, and in other modes (i.e. not mowing) it is -1.

Implementation

There might be a neater way of doing it, publishing in a more appropriate place etc., so I'm happy to be corrected. This is pretty much my first ever C++ compile and PR.

olliewalsh commented 3 months ago

is updating ntrip_client intentional?

11phc commented 3 months ago

is updating ntrip_client intentional?

it is not, no. I'm not sure where that came from. I will unwind.

11phc commented 3 months ago

ok i've cleaned it all up into this last commit, aebfb64, where all the changes should now be visible in one place.

olliewalsh commented 3 months ago

@ClemensElflein WDYT? @11phc for the submodules think you need to open PRs in the respective repos first then update this to use them

11phc commented 3 months ago

yes i was wondering about that. At least now people can understand what the motivation for doing so would be. Will await confirm from @ClemensElflein and then do those x2 PR's separately if all agreed.

ClemensElflein commented 2 months ago

Thank you for the PR, looks good to me. Also thank you @olliewalsh for the review.

Yes, changes to the submodules need to be PR'd separately and linked, I'll merge then in the correct order for this to work.

11phc commented 2 months ago

thanks @ClemensElflein, ok, will split them out for you. Might not be until the end of this week

11phc commented 2 months ago

i've now made the two linked PR requests now for xbot_msgs and xbot_monitoring modules, which need merging first.

https://github.com/ClemensElflein/xbot_msgs/pull/1 https://github.com/ClemensElflein/xbot_monitoring/pull/3

let me know if anything needs adjusting. cheers

ClemensElflein commented 1 month ago

Thank you for the PRs, I have merged the PRs, updated the references here and will merge this now.

vermut commented 4 weeks ago

Is this a new var? Should I update HA integration or it was already there?