Skip to content

Conversation

ShafiqSadat
Copy link

@ShafiqSadat ShafiqSadat commented Nov 8, 2023

Fixes #1208

Copy link
Contributor

@hamishwillee hamishwillee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks correct to me, as mission_item is deprecated. @peterbarker Do you know how the CI works - not reporting anything on this.

@hamishwillee
Copy link
Contributor

@morzack What do you think about this one?

@hamishwillee hamishwillee requested review from morzack and peterbarker and removed request for hamishwillee November 16, 2023 20:52
@hamishwillee
Copy link
Contributor

@ShafiqSadat I'm not going to merge this. That has to be done by someone with more technical knowledge. I have requested Peter and John, but this will have to await their attention.

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been wanting to do something like this twine thing for a very long time :-)

There are good changes in here, but you've got several unrelated changes mixed into the one PR, making merging anything difficult.

You should also only have a single commit per logical change, and commit messages which actually say what logical change has been made.

def _update_channel(self, channel, value):
# If we have channels on different ports, we expand the Channels
# object to support them.
channel = int(channel)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is strange

except APIException as e:
self._logger.exception('Exception in MAVLink input loop')
#self._logger.exception('Exception in MAVLink input loop')
self._logger.warning('%s' % str(e))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong

@ShafiqSadat ShafiqSadat requested a review from peterbarker June 1, 2024 05:15
self.notify_attribute_listeners('home_location', self.home_location, cache=True)

@self.on_message(['WAYPOINT', 'MISSION_ITEM'])
@self.on_message(['WAYPOINT', 'MISSION_ITEM', 'MISSION_ITEM_INT'])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a bug if It is a mission item int then you should divide to 1e7

self._master.mav.mission_item_send(0, 0, 0, frame,
self._master.mav.mission_item_int_send(0, 0, 0, frame,
mavutil.mavlink.MAV_CMD_NAV_WAYPOINT, 2, 0, 0,
0, 0, 0, location.lat, location.lon,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

conversion issu int(location.lat * 1e7), int(location.lon * 1e7)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warning: "got MISSION_REQUEST; use MISSION_REQUEST_INT!" when requesting missions with DroneKit
4 participants