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

boards: shields: add suport for st_b_cams_omv_mb1683 #76143

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

boards: shields: add suport for st_b_cams_omv_mb1683 #76143

wants to merge 1 commit into from
+232 −0

Conversation

CharlesDias
Copy link
Contributor

@CharlesDias CharlesDias commented Jul 21, 2024 •
edited
Loading

@CharlesDias CharlesDias marked this pull request as draft July 21, 2024 21:05
@zephyrbot zephyrbot added the area: Shields Shields (add-on boards) label Jul 21, 2024
This was referenced Jul 21, 2024
Open
Open
@CharlesDias CharlesDias force-pushed the st_b_cams_omv_mb1683 branch 2 times, most recently from 4377923 to 74f10a5 Compare July 22, 2024 20:44
@josuah josuah added area: Video Video subsystem area: camera labels Jul 22, 2024
erwango
erwango reviewed Jul 30, 2024
View reviewed changes
boards/shields/st_b_cams_omv_mb1683/doc/index.rst Outdated
Comment on lines 98 to 105
``_st_b_cams_omv_mb1683`` and adding the necessary device tree properties.

Set ``--shield "_st_b_cams_omv_mb1683"`` when you invoke ``west build``. For example:

.. zephyr-app-commands::
:zephyr-app: samples/subsys/video/capture
:board: stm32h7b3i_dk
:shield: _st_b_cams_omv_mb1683
Copy link
Member

Choose a reason for hiding this comment

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

Why using the prefix _ in _st_b_cams_omv_mb1683 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. It was a copy past problem! :-)

Thanks for your review! Let me know if you have any other suggestion.

Copy link
Member

Choose a reason for hiding this comment

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

For now PR was in draft, so I didn't go much further in the review.If not draft anymore, just move PR to ready for review.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm wanting to this #76124 going to upstream before change it to review.

@CharlesDias CharlesDias changed the title boards: shields: add suport for st_b_cams_omv_mb1683 [DNM] boards: shields: add suport for st_b_cams_omv_mb1683 Aug 5, 2024
@CharlesDias CharlesDias marked this pull request as ready for review September 14, 2024 21:49
@zephyrbot zephyrbot added the area: Samples Samples label Sep 14, 2024
@CharlesDias CharlesDias changed the title [DNM] boards: shields: add suport for st_b_cams_omv_mb1683 boards: shields: add suport for st_b_cams_omv_mb1683 Sep 14, 2024
@CharlesDias
Copy link
Contributor Author

Rebase.

@CharlesDias
Copy link
Contributor Author

Updated endpoint configuration according to #74415.

ngphibang
ngphibang reviewed Oct 14, 2024
View reviewed changes
boards/shields/st_b_cams_omv_mb1683/boards/stm32h7b3i_dk.overlay
* SPDX-License-Identifier: 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.

you forgot to include video-interfaces.h ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

boards/shields/st_b_cams_omv_mb1683/st_b_cams_omv_mb1683.overlay Outdated
*
*/

#include <zephyr/dt-bindings/video/video-interfaces.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

you don't need to include video-interfaces.h here as it does not use anything in this file (?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed. Thank you, @ngphibang!

samples/drivers/video/capture_to_lvgl/boards/stm32h7b3i_dk.conf Outdated

CONFIG_LOG_BUFFER_SIZE=2048

CONFIG_VIDEO_BUFFER_POOL_NUM_MAX=2
Copy link
Contributor

Choose a reason for hiding this comment

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

no need, the default value is 2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

@kartben
Copy link
Collaborator

kartben commented Nov 26, 2024

@CharlesDias will you be able to revisit this PR?

@CharlesDias
Copy link
Contributor Author

@CharlesDias will you be able to revisit this PR?

Hi, @kartben. Sure. I'll return this weekend. ;-)

@CharlesDias
boards: shields: add suport for st_b_cams_omv_mb1683
dba679b 
Add support for ST B-CAMS-OMV MB1683 shield.

Signed-off-by: Charles Dias <[email protected]>
@CharlesDias
Copy link
Contributor Author

Hi, @kartben and @ngphibang ,

I updated this PR, but it depends on #76124.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erwango erwango erwango left review comments

@ngphibang ngphibang ngphibang left review comments

@loicpoulain loicpoulain loicpoulain approved these changes

@avisconti avisconti Awaiting requested review from avisconti

@jfischer-no jfischer-no Awaiting requested review from jfischer-no

@kartben kartben Awaiting requested review from kartben

@josuah josuah Awaiting requested review from josuah

@nashif nashif Awaiting requested review from nashif

At least 2 approving reviews are required to merge this pull request.

Assignees

@kartben kartben

Labels
area: camera area: Samples Samples area: Shields Shields (add-on boards) area: Video Video subsystem
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@CharlesDias @kartben @ngphibang @loicpoulain @erwango @zephyrbot @josuah