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.

Sign up for GitHub

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

Jump to bottom

Ur 10 #7

Merged
merged 11 commits into from
Nov 11, 2021
Merged

Ur 10 #7

merged 11 commits into from
Nov 11, 2021

Conversation

yagizyalcintas
Copy link
Contributor

No description provided.

@yagizyalcintas
Create readme.md
aa5b133 
add readme
@yagizyalcintas
Add files via upload
7a64096 
add 
constants.json, requirements.txt, td.json, ur10_flask.py, urTD.py
@yagizyalcintas
Create config.json
e047ab0 
add config.json under TM2TD
@yagizyalcintas
Update config.json
6608caf 
enter placeholders in config.json
@yagizyalcintas
Add files via upload
da5369a 
add convertedTD.json, replace.py, TM.json
@egekorkan
Copy link
Member

egekorkan commented Oct 10, 2021 •
edited
Loading

I have submitted my review. Some additional comments:

  • A readme is desperately needed.
  • Why is there a td.json and also a convertedTD.json ?
  • Why is the TD/TM duplicated in urTD.py ? Together with the previous comment, I know need to change/maintain 3 documents? Which ones are used by the server to expose td?
  • Why is the connection to the robot established at each affordance?
  • There is copy paste leftover from scrollphat...

egekorkan
egekorkan requested changes Oct 10, 2021
View reviewed changes
Devices/UR10-py/TM2TD/replace.py Outdated
config_new = {}

for keyy in config:
anahtar = r"{{" + str(keyy) + r"}}"
Copy link
Member

Choose a reason for hiding this comment

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

english please

Devices/UR10-py/TM2TD/replace.py Outdated

config_new = {}

for keyy in config:
Copy link
Member

Choose a reason for hiding this comment

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

double y

Devices/UR10-py/TM2TD/replace.py Outdated
#print(tm_modified_dict)


tm_modified_dict["@type"] = "UR-10 Robot Arm"
Copy link
Member

Choose a reason for hiding this comment

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

replace.py should be generic

Devices/UR10-py/TM2TD/replace.py Outdated


tm_modified_dict["@type"] = "UR-10 Robot Arm"
tm_modified_dict["securityDefinitions"] = {"nosec_sc": {"scheme": "nosec"}}
Copy link
Member

Choose a reason for hiding this comment

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

this is our choice, not generic

Devices/UR10-py/TM2TD/replace.py Outdated
"op": "readproperty",
"contentType":"application/json",
"htv:methodName": "GET",
"security": "nosec_sc"
Copy link
Member

Choose a reason for hiding this comment

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

why security at the form level?

Devices/UR10-py/ur10_flask.py Outdated
jointPoslist.append(list1[i+3])
print(jointPoslist)
for i in range (6):
jointPoslist[i]= jointPoslist[i]/57.29
Copy link
Member

Choose a reason for hiding this comment

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

what is this number?

Devices/UR10-py/ur10_flask.py Outdated Show resolved Hide resolved
Devices/UR10-py/ur10_flask.py Outdated
return "robot is not in Normal mode"
abort(400)
else:
return "Error 415"
Copy link
Member

Choose a reason for hiding this comment

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

do you actually enter the abort if you do a return before?

Devices/UR10-py/ur10_flask.py Outdated
dur = 7
count = 0
while count < dur*(10):
scrollphathd.clear()
Copy link
Member

Choose a reason for hiding this comment

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

well well well...

Devices/UR10-py/ur10_flask.py Outdated
while True:
try:
# connect to router to ensure a successful connection
s.connect(('172.16.1.1', 80))
Copy link
Member

Choose a reason for hiding this comment

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

should be constant that can be changed from the outside

egekorkan
egekorkan requested changes Oct 12, 2021
View reviewed changes
Devices/UR10-py/replace.py Show resolved Hide resolved
Devices/UR10-py/TM.json Outdated
},
"readOnly": true
},
"curLocation":{
Copy link
Member

Choose a reason for hiding this comment

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

homeloc and curloc have different schemas. even though it is technically possible, it feels weird. homeloc is in joint coordinates whereas this one is in cartesian. It should be better named. I suggest renaming homeLoc to homeJointPositions

Devices/UR10-py/replace.py Outdated
"op": "readproperty",
"contentType":"application/json",
"htv:methodName": "GET"

Copy link
Member

Choose a reason for hiding this comment

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

better to not have any unnecessary empty lines

Devices/UR10-py/ur10_flask.py Outdated

@app.route("/ur10/properties/curJointPos", methods=["GET"])
def curJointPos():
##rtde_c = RTDEControl("172.16.1.222")
Copy link
Member

Choose a reason for hiding this comment

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

simply remove rather than commenting :)

Devices/UR10-py/ur10_flask.py Outdated

app = Flask(__name__)

rtde_c = RTDEControl("172.16.1.222")
Copy link
Member

Choose a reason for hiding this comment

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

Should be configurable over configurations

Devices/UR10-py/ur10_flask.py Outdated
abort(415,"Error 415")


##################################################3
Copy link
Member

Choose a reason for hiding this comment

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

a sneaky 3

Devices/UR10-py/ur10_flask.py Outdated

abort(415,"Error 415")

###################################################333
Copy link
Member

Choose a reason for hiding this comment

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

sneaky 3 3s

Devices/UR10-py/ur10_flask.py Outdated
@app.route("/ur10/actions/gripClose", methods=["POST"])
def gripClose():

rtde_io_ = RTDEIO("172.16.1.222")
Copy link
Member

Choose a reason for hiding this comment

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

leftover

Devices/UR10-py/ur10_flask.py Outdated
@app.route("/ur10/actions/gripOpen", methods=["POST"])
def gripOpen():

rtde_io_ = RTDEIO("172.16.1.222")
Copy link
Member

Choose a reason for hiding this comment

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

leftover

Devices/UR10-py/replace.py Outdated
file.write(json.dumps(tm_modified_dict, indent=1, sort_keys=True))


print(json.dumps(tm_modified_dict, indent=1, sort_keys=True))
Copy link
Member

Choose a reason for hiding this comment

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

not needed?

egekorkan
egekorkan requested changes Oct 16, 2021
View reviewed changes
Devices/UR10-py/TM.json Outdated
@@ -42,7 +42,7 @@
},
"readOnly": true
},
"curLocation":{
"currentCoordinates":{
"title":"Location",
Copy link
Member

Choose a reason for hiding this comment

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

needs to reflect the affordance name

Devices/UR10-py/TM.json Outdated
@@ -66,7 +66,7 @@
},
"readOnly": true
},
"curJointPos":{
"currentJointDegrees":{
"title":"Current Joint Position",
Copy link
Member

Choose a reason for hiding this comment

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

needs to reflect the affordance name

Devices/UR10-py/readme.md Outdated
* Flask
* jsonschema
* Pillow
Copy link
Member

Choose a reason for hiding this comment

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

Needs to be removed

Devices/UR10-py/requirements.txt Outdated
@@ -2,4 +2,4 @@ Flask==1.0.2
requests==2.18.4
jsonschema==3.2.0
Pillow==8.3.1
Copy link
Member

Choose a reason for hiding this comment

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

needs to be removed

Devices/UR10-py/ur10_flask.py Outdated
@app.route("/ur10/properties/homeloc", methods=["GET"])
def homeloc():
x = json.dumps(HOMELOCATION)
print(type(json.dumps(HOMELOCATION)))
Copy link
Member

Choose a reason for hiding this comment

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

Still not resolved. There are double logs.

@egekorkan egekorkan merged commit 8e3d89f into tum-esi:master Nov 11, 2021
@egekorkan egekorkan mentioned this pull request Nov 11, 2021
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@egekorkan egekorkan egekorkan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yagizyalcintas @egekorkan