Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"TP: Robot_Servicio, Alumnos: Bonn y Castellaro" #198

Open
wants to merge 3 commits into
base: kinetic-devel
Choose a base branch
from

Conversation

fedecastellaro
Copy link

No description provided.

@fedecastellaro fedecastellaro requested a review from eborghi10 as a code owner June 8, 2020 21:04
@eborghi10
Copy link
Member

Es considerada buena práctica poner una descripción en el PR con instrucciones sobre cómo ejecutarlo y probar los cambios que agregaron. @fedecastellaro


class send_goals():
def __init__(self):
self.pi = 3.1416
Copy link
Member

Choose a reason for hiding this comment

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

En Python puede usar esto:

import math

print(math.pi)

Fuente: https://docs.python.org/2/library/math.html#constants

def __init__(self):
self.pi = 3.1416

sec_1_table_1 = Pose()
Copy link
Member

Choose a reason for hiding this comment

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

Algo que estaria bueno considerar pero no sé cuan bien funciona, sería usar algo así como "base de datos" y el robot puede ir leyend esas poses: https://strands.readthedocs.io/en/latest/strands_navigation/topological_navigation/#creation-of-the-topological-map-from-waypoint-file

Copy link
Member

Choose a reason for hiding this comment

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

Esto también se los mencioné en el anteproyecto y creo que sería un muy buen feature: https://strands.readthedocs.io/en/latest/lamor15/wiki/Tutorial-materials-1.html#creating-the-topological-map

@@ -0,0 +1,67 @@
#!/usr/bin/env python
Copy link
Member

Choose a reason for hiding this comment

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

Existe un paquete que ya hace esto por ustedes: http://wiki.ros.org/follow_waypoints

Tal vez puedan integrarlo para lograr hacer algo más complejo.

El código está acá: https://github.com/danielsnider/follow_waypoints

Copy link
Member

Choose a reason for hiding this comment

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

De hecho, si ven el código, se pueden fijar que lee un CSV como les propuse en otro comentario.

Copy link
Member

Choose a reason for hiding this comment

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

Se puede eliminar este archivo teniendo en cuenta que están usando el otro paquete.

Copy link
Member

Choose a reason for hiding this comment

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

Lo mismo para el archivo scanf.py.

@eborghi10 eborghi10 marked this pull request as draft June 8, 2020 22:14
Alumnos: Bonn y Castellaro
@eborghi10 eborghi10 marked this pull request as ready for review July 18, 2020 16:09
@eborghi10
Copy link
Member

eborghi10 commented Jul 18, 2020

@fedecastellaro Podrías actualizar el código con las últimas modificaciones?

git pull origin kinetic-devel

Donde origin apunta a RoboticaUtnFrba/create_autonomy.

Vas a tener conflictos con archivos que fueron modificados en ambos lados.

Imagino que ese conflicto podés evitarlo eliminando los archivos .rviz.

@@ -0,0 +1,18 @@
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Changelog for package follow_waypoints
Copy link
Member

Choose a reason for hiding this comment

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

Ojo que si usan código hecho por otra persona, deberían agregar la licencia correspondiente.

Copy link
Member

Choose a reason for hiding this comment

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

No hay problema con la licencia, pero este archivo no creo que sea necesario.

@@ -0,0 +1,47 @@
# follow_waypoints [![Build Status](http://build.ros.org/buildStatus/icon?job=Kbin_uX64__follow_waypoints__ubuntu_xenial_amd64__binary)](http://build.ros.org/job/Kbin_uX64__follow_waypoints__ubuntu_xenial_amd64__binary)
Copy link
Member

Choose a reason for hiding this comment

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

Editen este archivo para que le sea útil a quien quiera usarlo.

@@ -0,0 +1,8 @@
<launch>
<env name="ROSCONSOLE_FORMAT" value="[${severity}][${thread}][${node}/${function}:${line}]: ${message}"/>
Copy link
Member

Choose a reason for hiding this comment

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

Interesante esta variable de entorno, no la conocía.

@@ -0,0 +1,7 @@
image: map.pgm
Copy link
Member

Choose a reason for hiding this comment

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

Los archivos de map a qué entorno corresponde? Pueden eliminarse?

@@ -0,0 +1,199 @@
#!/usr/bin/env python
Copy link
Member

Choose a reason for hiding this comment

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

Hicieron algún cambio en este paquete? Aún así, puedo hacer un fork del repositorio original así no es necesario copiarlo dentro de create_autonomy.

<!-- One license tag required, multiple allowed, one license per tag -->
<!-- Commonly used license strings: -->
<!-- BSD, MIT, Boost Software License, GPLv2, GPLv3, LGPLv2.1, LGPLv3 -->
<license>TODO</license>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<license>TODO</license>
<license>BSD</license>

Comment on lines +19 to +40
<!-- Url tags are optional, but multiple are allowed, one per tag -->
<!-- Optional attribute type can be: website, bugtracker, or repository -->
<!-- Example: -->
<!-- <url type="website">http://wiki.ros.org/robot_servicio</url> -->


<!-- Author tags are optional, multiple are allowed, one per tag -->
<!-- Authors do not have to be maintainers, but could be -->
<!-- Example: -->
<!-- <author email="[email protected]">Jane Doe</author> -->


<!-- The *depend tags are used to specify dependencies -->
<!-- Dependencies can be catkin packages or system dependencies -->
<!-- Examples: -->
<!-- Use depend as a shortcut for packages that are both build and exec dependencies -->
<!-- <depend>roscpp</depend> -->
<!-- Note that this is equivalent to the following: -->
<!-- <build_depend>roscpp</build_depend> -->
<!-- <exec_depend>roscpp</exec_depend> -->
<!-- Use build_depend for packages you need at compile time: -->
<!-- <build_depend>message_generation</build_depend> -->
Copy link
Member

Choose a reason for hiding this comment

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

Eliminen todo lo que sea código comentado.

Suggested change
<!-- Url tags are optional, but multiple are allowed, one per tag -->
<!-- Optional attribute type can be: website, bugtracker, or repository -->
<!-- Example: -->
<!-- <url type="website">http://wiki.ros.org/robot_servicio</url> -->
<!-- Author tags are optional, multiple are allowed, one per tag -->
<!-- Authors do not have to be maintainers, but could be -->
<!-- Example: -->
<!-- <author email="[email protected]">Jane Doe</author> -->
<!-- The *depend tags are used to specify dependencies -->
<!-- Dependencies can be catkin packages or system dependencies -->
<!-- Examples: -->
<!-- Use depend as a shortcut for packages that are both build and exec dependencies -->
<!-- <depend>roscpp</depend> -->
<!-- Note that this is equivalent to the following: -->
<!-- <build_depend>roscpp</build_depend> -->
<!-- <exec_depend>roscpp</exec_depend> -->
<!-- Use build_depend for packages you need at compile time: -->
<!-- <build_depend>message_generation</build_depend> -->

def __init__(self):
self.new_pose = Pose()
self.received_goals = []
rospy.Subscriber("/create1/move_base_goals", String, self.register_goal)
Copy link
Member

Choose a reason for hiding this comment

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

Este approach no va a funcionar con múltiples robots, por lo cual tienen que eliminar create1 y agregar el namespace al launch file.

Suggested change
rospy.Subscriber("/create1/move_base_goals", String, self.register_goal)
rospy.Subscriber("move_base_goals", String, self.register_goal)

@@ -0,0 +1,67 @@
#!/usr/bin/env python
Copy link
Member

Choose a reason for hiding this comment

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

Se puede eliminar este archivo teniendo en cuenta que están usando el otro paquete.

@@ -0,0 +1,67 @@
#!/usr/bin/env python
Copy link
Member

Choose a reason for hiding this comment

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

Lo mismo para el archivo scanf.py.

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.

2 participants