Distance sensor capabilities

xabierolaz commented 4 years ago

Requirements for Pull Request

Read CONTRIBUTING.md.
Open an issue. Any change needs to be discussed before proceeding. Failure to do so may result in the rejection of the pull request.
Fill out the template below, any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.

Description of the Change

1-Added message definition for the sensor by updating the state protobuff message

Verification Process

Has not been verified yet, as it's just the capability of adding the distance sensor into gymfc

Tested by running test_axis.py

No errors in console. However, sensor does not seem to appear in Gazebo yet inside the nf1 model

wil3 commented 4 years ago

I suggest for this PR you scope it to just the core GymFC changes, so in addition to the state.proto this includes modifications to the cpp plugin https://github.com/wil3/gymfc/blob/master/gymfc/envs/assets/gazebo/plugins/FlightControllerPlugin.cpp. You can use IMU or ESC as an example.

A separate PR should be used to introduce the implementation in an actual model.

xabierolaz commented 4 years ago

I suggest for this PR you scope it to just the core GymFC changes, so in addition to the state.proto this includes modifications to the cpp plugin https://github.com/wil3/gymfc/blob/master/gymfc/envs/assets/gazebo/plugins/FlightControllerPlugin.cpp. You can use IMU or ESC as an example.

A separate PR should be used to introduce the implementation in an actual model.

Will add cpp plugin to this PR and commit sdf model in a separate PR as you stated

xabierolaz commented 4 years ago

I suggest for this PR you scope it to just the core GymFC changes, so in addition to the state.proto this includes modifications to the cpp plugin https://github.com/wil3/gymfc/blob/master/gymfc/envs/assets/gazebo/plugins/FlightControllerPlugin.cpp. You can use IMU or ESC as an example.

A separate PR should be used to introduce the implementation in an actual model.

@wil3 distance sensor should be under the IMU category right?

wil3 commented 4 years ago

removed "repeated" as its single value

I would keep this as repeated, it gives you flexibility to return distances in the future for different directions. It would be useful for navigation to know the distance around you, not just below you.

What I meant by my previous comment is that a comment should be added to explain to the user the different indices could represent distances in different directions.

wil3 commented 4 years ago

I suggest for this PR you scope it to just the core GymFC changes, so in addition to the state.proto this includes modifications to the cpp plugin https://github.com/wil3/gymfc/blob/master/gymfc/envs/assets/gazebo/plugins/FlightControllerPlugin.cpp. You can use IMU or ESC as an example. A separate PR should be used to introduce the implementation in an actual model.

@wil3 distance sensor should be under the IMU category right?

Not sure what you mean by IMU category, can you be more specific?

xabierolaz commented 4 years ago

I suggest for this PR you scope it to just the core GymFC changes, so in addition to the state.proto this includes modifications to the cpp plugin https://github.com/wil3/gymfc/blob/master/gymfc/envs/assets/gazebo/plugins/FlightControllerPlugin.cpp. You can use IMU or ESC as an example. A separate PR should be used to introduce the implementation in an actual model.

@wil3 distance sensor should be under the IMU category right?

Not sure what you mean by IMU category, can you be more specific?

@wil3 what I mean is if for having working in GymFC the distance sensor has to be in the FlightControllerPlugin.cpp added as an imu or as an esc value inside the code, or if its a totally different category like this

#include "MotorCommand.pb.h"
#include "EscSensor.pb.h"
#include "Imu.pb.h"
#include "State.pb.h"
#include "Action.pb.h"
#include "Distance.pb.h"<<<<<

Another example of what I'm talking about

enum Sensors {
    IMU,
    ESC,
    BATTERY
    DISTANCE << is this way ok or distance should be considered as an Imu or Esc sensor 
  };

I'm creating a separate PR for the actual implementation of the model as you suggested

wil3 / gymfc