Skip to content
This repository has been archived by the owner on Aug 3, 2021. It is now read-only.

Sending a new packet type to commander_generic #79

Closed
malintha opened this issue Dec 9, 2017 · 5 comments
Closed

Sending a new packet type to commander_generic #79

malintha opened this issue Dec 9, 2017 · 5 comments

Comments

@malintha
Copy link

malintha commented Dec 9, 2017

Hi @whoenig ,

How can we set the packet type value in the setpoint request? I am trying to modify the code in order to send a new packet type as mentioned in cf firmware and just couldn't figure out the place you have set the packet type.

Issue for the new commander_generic implementation
bitcraze/crazyflie-firmware#113

@whoenig
Copy link
Owner

whoenig commented Dec 9, 2017

There is some generic packet support, but it would be better to add native support:

  1. add the packet type here: https://github.com/whoenig/crazyflie_ros/blob/master/crazyflie_cpp/include/crazyflie_cpp/crtp.h#L187-L206.
  2. add a function (could be overloaded): https://github.com/whoenig/crazyflie_ros/blob/master/crazyflie_cpp/src/Crazyflie.cpp#L118-L126
  3. add ROS support in https://github.com/whoenig/crazyflie_ros/blob/master/crazyflie_driver/src/crazyflie_server.cpp

I'll do that eventually, but PRs are welcome!

@malintha
Copy link
Author

malintha commented Dec 9, 2017

Hi @whoenig,

More than happy to contribute. A question though;

If we are to implement a packet format that complies with the new cf generic packet format, I feel like all we have to do is to add another header param for the packet type to setPointPacket. (Referring to this diagram - bitcraze/crazyflie-firmware#113 (comment)). Am I correct?

@malintha
Copy link
Author

malintha commented Dec 9, 2017

@whoenig,

AFAIU, the current ROS cf packet calls to the port 0x03 and channel 0. But in order to comply with the new generic packetDecoder approach, we have to call channel 0x07. (as per here: https://github.com/bitcraze/crazyflie-firmware/blob/c55ed40b05b370e73ae85c957636aac78c87f20a/src/modules/src/crtp_commander.c#L58 )

So is it correct to say we need to overload the crtp constructer if we are to add a new packet type with the new header values?

@whoenig
Copy link
Owner

whoenig commented Dec 14, 2017

I think there should be a new crtp type that uses a union inside (with one struct for each type), see https://github.com/bitcraze/crazyflie-firmware/blob/master/src/modules/src/crtp_commander_generic.c for a full list.

Perhaps, something like this:

struct crtpGenericSetpointRequest
{
  crtpGenericSetpointRequest(
    /*params for type1*/)
    : header(0x07, 0)
  /* initialize relevant union*/
  {
  }
  const crtp header;
  uint8_t type;
  union data {
    struct stop {
    } __attribute__((packed));
    struct velocity {
      float vx;        // m in the world frame of reference
      float vy;        // ...
      float vz;        // ...
      float yawrate;  // deg/s
     } __attribute__((packed));
  /* other types here*/
  };
} __attribute__((packed));

Another option would be to add a specific crtp packet for each of the possible "generic" types (avoiding confusing overloaded constructors).

@whoenig
Copy link
Owner

whoenig commented Apr 18, 2018

Most generic setpoints are implemented now (e.g., cmd_hover, cmd_position, and cmd_full_state).

@whoenig whoenig closed this as completed Apr 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants