-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feature: RGB Led controlled via ROS #2
Conversation
- still using Serial Connection --> should consider moving to ROS publisher for RGB. Can publish an array of the RGB values and send that to the Arduino script to input the values to the RGB - Will test on 4/4
- Need to use nested For loop to go through all RGB image values - Might need to do a ROS connection. Publish the RGB values read from the image to the arduino script rather than serial connection
- Already able to read pixel values using img[0][0] command - NEXT: How to send the pixel data to the Arduino Serial Connection tutorials aren't clear on how to send multiple integers to RGB - Could publish RGB values into a topic then have Arduino script pull that from the rgb topic
* Use `/arduino_control` as Arduino Sketchbook folder for compiling * Individual files were not reviewed
* add libraries callout * add libraries .gitignore * add README.md for arduino controller codes
* Add RGB led control interface. Compiels, but untested on hardware. * Add custom message for RGB led state * Update CMakeLists and Package.xml for new message
* add launch file with argument for port * locked Arduino Baud rate at 500,000
Next StepsSetup
Main
Pseudo CodeThis might look something like this pseudo-ish code. import rospy
from light_painting.msg import RGBState
pub = rospy.Publisher('paintbrush_color ', RGBState, queue_size=5)
msg = RGBState()
msg.red = 187
msg.green = 0
msg.blue = 0
pub.publish(msg) |
- USE RGB_values_publisher to test the publisher - Added nested for loop to RGB_no_descartes script - NEED TO TEST IF ARDUINO READS THE RGB VALUES SENT FROM PUBLISHER
-Keep losing sync with arduino -have not successfully used rgb publisher in robot_motion script
To launch: just run light_painting.launch. it already initializes the ARduino connection. - Added Block O RGB - Occasionally gets Timeout for Trajectory motion for large images (10x10)
- Removed comments, notes, unused variables in RGB_no_descartes script - Added grayscale images for next feature
* reformat * remove extra scraps and notes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the commits! Overall, this looks pretty good. I made a few suggestions and requested a few changes to help overall clarity of the code. Comments are here as well as scattered throughout the code.
Please make those changes and then this should be ready. Changes can just be pushed to the same branch again.
General Comments:
- Suggest removing extra image files that are not directly used by the project.
- Please improve naming convention for python scripts or provide a README. The multiple
py_to_ino
... andno_descartes
... scripts are confusing. - Unclear which file is the main file to start. No ros
.launch
file to automatically select the correct file either.
@@ -11,7 +11,7 @@ | |||
BINARY = join(RESOURCES,'binary') # combine image folder location with binary | |||
# print('Binary Directory:',BINARY) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove extra print()
statements that were used for original debugging.
# BGR = cv2.imread(join(R_G_B,blockO_rgb)) | ||
# RGB = cv2.cvtColor(BGR,cv2.COLOR_BGR2RGB) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only keep a single line that's being run. Don't duplicate.
This is convenient when doing your own debugging/testing, but shouldn't be kept in final version.
Remove this and other duplicated & commented out sections (ie. the above blockO_rgb line as well).
scripts/py_to_ino_RGB_LED_test.py
Outdated
# arduino = serial.Serial(port='/dev/ttyACM0',baudrate=9600,timeout=0.1) | ||
# Code from: https://www.electronicshub.org/controlling-arduino-led-python/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove all serial
commands from script since we're using the roslaunch & rosserial package to communicate with Arduino.
# print('Size of RGB: ',rgb_img.size) | ||
# print('rgb_img[0,0] pixel 1 value:',rgb_img[0,0]) | ||
# print('rgb_img.item(0) pixel 1 value:',rgb_img.item(0)) # gets only the 1st value | ||
# print('rgb_img[0,0] pixel 4 value(mid pixel):',rgb_img[1,1]) | ||
# print('rgb_img[0,0] pixel 8 value(bottom right):',rgb_img[2,2]) | ||
# red,green,blue = rgb_img[1,1] | ||
# print('Red value of pixel 4:',red) | ||
# print('Green value of Pixel 4:',green) | ||
# print('Blue value of pixel 4:',blue) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this block of print statements. Not used by the actual code.
Not helpful for debugging.
Suggest adding a sentence or two explaining what this function does, you already kind of have it with the top comment "Testing reading values....". Maybe just add what the input value needs to be and what the function returns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comment about the specific order of RGB vs BGR may be good to include since you mention it a few lines below as well.
class RGB_publisher(): | ||
def __init__(self,rgb_values): | ||
self.rgb_values = rgb_values | ||
def runner(self,data): | ||
try: | ||
rgb_values = [0,0,0] | ||
rgb_img = input_image.rgb | ||
|
||
|
||
r,g,b = RGB_values(rgb_img) | ||
data = (r,g,b) | ||
|
||
rgb_values.data = list(bytearray(data)) | ||
|
||
self.rgb_values.publish(rgb_values) | ||
rospy.loginfo(rgb_values) | ||
|
||
|
||
except rospy.ROSInterruptException: | ||
exit() | ||
except KeyboardInterrupt: | ||
exit() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't understand why class is at the bottom of the file.
Suggest moving above main()
function to more consistently organize code like other python scripts.
print('Keep moving horizontally') | ||
waypoints = nextColumn(wpose) | ||
|
||
# input(f"Cartesian Plan {i}: press <enter>") # uncomment this line if you want robot to run automatically |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this comment as was used for debugging and would not be used in final code.
plan, fraction = rc.plan_cartesian_path(waypoints) | ||
rc.execute_plan(plan) | ||
sendRGB2LED(pub_rgb_values,r,g,b) | ||
time.sleep(0.5) | ||
sendRGB2LED(pub_rgb_values) | ||
time.sleep(0.5) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest adding a comment on why send value, sleep, then send again.
The practical function of these lines of code are vague, but critical to operation of this script. Add documentation.
* gitignore all .pyc files * misc code formatting * remove extra/unused imports
@MohammadKhan-3 Left a review of code. See comments in the review. Once changes made, I think this is ready to merge. |
Built the rosserial_arduino library I ran
To create the Binary publisher:
|
084047d
to
24a4c52
Compare
Rolled back three commits to have this PR only include RGB. Moving Greyscale feature onto a new branch following this PR. |
RGB Led onboard an Arduino Uno is controllable via ROS. PR updates Arduino Sketchbook.
Major Updates
/paintbrush_color
topicTests
Launched control to Arduino using
roslaunch light_painting t2_led_paintbrush.launch
.Used RQT's Message Publisher to publish test messages and confirmed LED updated to match RGB setting.
Design Notes:
Verified Arduino had sufficient buffer size on Arduino Uno. Message is 3xUINT8 values (ie. 3 bytes). Current program size leaves ~600 bytes of memory available for variables. This is sufficient.
Program Control Flow:
Remaining Work