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

esp32-camera #26

Open
wants to merge 17 commits into
base: foxy
Choose a base branch
from
Open

Conversation

DorBenHarush
Copy link

@DorBenHarush DorBenHarush commented Aug 11, 2020

  • Example app for publishing information from camera

  • add camera option to menuconfig and allowing it.

Signed-off-by: Dor Ben Harush [email protected]

* Example app for publishing information from camera

* add camera option to menuconfig and allowing it.
@ralph-lange
Copy link
Contributor

Thank you very much for your contribution, @DorBenHarush. In order for this pull request to be merged, please sign-off your commit (git commit --amend --signoff) and add another commit in which you add your name to the NOTICE file in the root of the repository. Please also pay attention to the notes in NOTICE and CONTRIBUTING.md regarding the ownership of the code you are contributing.

@ralph-lange
Copy link
Contributor

Don't forget the sign-off tag, please, which requires a force push (git push -f) as you changed the commit messages.

@donRaphaco
Copy link

relates to micro-ROS/micro_ros_setup/pull/176

Copy link

@donRaphaco donRaphaco left a comment

Choose a reason for hiding this comment

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

I tested the example on my esp32-cam board and had no problems there.

Comment on lines 6 to 7
#define BOARD_WROVER_KIT
#define BOARD_ESP32CAM_AITHINKER

Choose a reason for hiding this comment

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

Is it possible to add an option in menuconfig where one board is selected?

Copy link
Author

Choose a reason for hiding this comment

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

According to espressif the only difference is in the sdkconfig.defaults file.
In this line : CONFIG_ESP32_SPIRAM_SUPPORT=y
the line can be added menually like I did or it can be added in the menuconfig (Enable PSRAM).
Src: https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/build-system.html#selecting-idf-target
and https://github.com/espressif/esp32-camera

Choose a reason for hiding this comment

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

Sorry I should have been more precise.
What I mean is that there are two defines here #define BOARD_WROVER_KIT and #define BOARD_ESP32CAM_AITHINKER they define which camera pinout to use (see Lines 35-79 ) and should be mutually exclusive.
Therefore I suggest to add an option to menuconfig where one of the two can be selected, which would also make it easier to add a new camera pinout for example the ESP-EYE development board.

Copy link
Author

@DorBenHarush DorBenHarush Aug 12, 2020

Choose a reason for hiding this comment

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

Check my last commit I added the option to choose camera pinout.

@@ -0,0 +1,197 @@
/**
* This example takes a picture every 5s.
Example src: https://github.com/espressif/esp32-camera.git

Choose a reason for hiding this comment

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

@ralph-lange parts from this example come from the example here. Do we need to mention that in 3rd-party-licenses.txt?
The license there is also Apache 2.0

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, all details (version, copyright statement, original license text) have to be provided in the 3rd-party-licenses.txt file, please. Furthermore, a short summary (one list item with one or two sentences) should go into the License section in README.md.

* Now it is possible to choose camera pinout in the menuconfig

Signed-off-by: Dor Ben Harush [email protected]
apps/take_picture/app.c Outdated Show resolved Hide resolved
apps/take_picture/app.c Outdated Show resolved Hide resolved
apps/take_picture/app.c Outdated Show resolved Hide resolved
rclc_executor_spin_some(&executor, 100);
usleep(100000);
//takes picture
pic = esp_camera_fb_get();
Copy link
Member

Choose a reason for hiding this comment

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

Indentation

@DorBenHarush
Copy link
Author

DorBenHarush commented Sep 1, 2020

Now, after this issue is solved i intend to create a new rclc_demo for the esp32camera.
I'll change the program here to an image publisher instead of image_size and the rclc_demo will receive the images.
I'll commit the changes in a few days.

suggestions and advices are welcomed.

@pablogs9
Copy link
Member

pablogs9 commented Sep 21, 2020

Hello @DorBenHarush this the last commit solves micro-ROS/micro_ros_setup#187?

Please be aware that RMW_UXRCE_STREAM_HISTORY is related to the size of the middleware buffer and RMW_UXRCE_MAX_HISTORY only sets the available slots of the RMW incoming buffers.

@DorBenHarush
Copy link
Author

Hi @pablogs9, No this coomit raises the issue. I have noticed that RMW_UXRCE_MAX_HISTORY matchs with the value inside colcon.meta while UXR_CONFIG_UDP_TRANSPORT_MTU doesnt.

this line : CONFIG_ESP32_SPIRAM_SUPPORT=y create a problem while creating firmware for esp32. 
Enable SPIRAM in `menuconfig`:  Component config > ESP32-specific
@DorBenHarush
Copy link
Author

DorBenHarush commented Oct 28, 2020

I have decided to change the pixel_format to jpeg to increase the resulution. In order to view the images I created a cv_bridge.
bridge:

#!/usr/bin/env python3
import rclpy
from rclpy.node import Node
from cv_bridge import CvBridge, CvBridgeError
from sensor_msgs.msg import CompressedImage
import cv2
import numpy as np




class MinimalSubscriber(Node):

    def __init__(self, ):
        super().__init__('minimal_subscriber')
        self.subscription = self.create_subscription(CompressedImage, 'freertos_picture_publisher', self.listener_callback, 10)
        self.subscription  # prevent unused variable warning 
        self.bridge = CvBridge()
    def listener_callback(self, image_message):
        self.get_logger().info('recieved an image')
        
    	#recieve image and co nvert to cv2 image
        cv2.imshow('esp32_image', self.cv_image)
        cv2.waitKey(3)

        


def main(args=None):
    rclpy.init(args=args)
    minimal_subscriber = MinimalSubscriber()
    
    rclpy.spin(minimal_subscriber)
    minimal_subscriber.destroy_node()
    rclpy.shutdown()

if __name__ == '__main__':
   main()`

endmenu


menu "Camera configuration"
Copy link
Member

Choose a reason for hiding this comment

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

Is there a possibility that we can set this as an application-specific configuration?

Copy link
Author

Choose a reason for hiding this comment

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

I have opened a PR for this here. if approved the full camera configurion will be inside the menuconfig only when esp32-camera repository is in the components directory (esp-idf/components).

@@ -1,3 +1,4 @@
CONFIG_ESP_MAIN_TASK_STACK_SIZE=3000
CONFIG_FREERTOS_UNICORE=y
CONFIG_ESP_TASK_WDT=n
CONFIG_RTCIO_SUPPORT_RTC_GPIO_DESC=y
Copy link
Member

Choose a reason for hiding this comment

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

Is there a possibility that we can set this as an application-specific configuration?

Copy link
Member

Choose a reason for hiding this comment

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

Related to: micro-ROS/micro_ros_setup#176 (comment)

Maybe we can just explain how to configure this in the app README.md.

Copy link
Author

Choose a reason for hiding this comment

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

I'll write a detailed REAMDE.md and add it to app.

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.

4 participants