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

Fix of unknown Ringbuffer and not found USB-Serial Device #125

Merged
merged 11 commits into from
Oct 26, 2023
Merged

Fix of unknown Ringbuffer and not found USB-Serial Device #125

merged 11 commits into from
Oct 26, 2023

Conversation

scrapforge
Copy link
Contributor

I added in prj.conf a CONFIG_RING_BUFFER=y line, due to an error while building, which stated an undefined reference occured, similar to this on zephyrproject-rtos/zephyr#30183.
Also i could not reliably reproduce the error. This line fixed any appearance of it.

Also if usb-serial configured as transport layer for microros, the code hang up. After a lot of debugging i found that in the file mymicromodule/modules/libmicroros/microros_transports/serial-usb/microros_transports.c the device is searched by its label CDC_ACM_0. Therefore an app.overlay file is needed, which i added.

I have used
zephyr 3.5.0-rc3
and
zephyr-sdk 0.16.3

@pablogs9
Copy link
Member

I added in prj.conf a CONFIG_RING_BUFFER=y line, due to an error while building, which stated an undefined reference occured, similar to this on zephyrproject-rtos/zephyr#30183.

Could you provide details on how to trigger this build error?

Also, the CI seems to have errors

@scrapforge
Copy link
Contributor Author

scrapforge commented Oct 24, 2023

The CI for Zephyr 2.7.2 complains about:
Error: disco_l475_iot1.dts.pre.tmp:1921.1-13 Label or path zephyr_udc0 not found
FATAL ERROR: Syntax error parsing input tree.

I considered the error could be based on an api/syntax change between the versions 2.7.2 and 3.1 but as i searched the documentation for Zephyr 2.7.2 i found, that the added overlay code is the same as proposed in
https://docs.zephyrproject.org/2.7.2/reference/usb/uds_cdc_acm.html?highlight=zephyr_udc0.

So i compared the device-tree overlays disco_l475_iot1.dts for the board and got (just the important detail):

-  &usbotg_fs {
+  zephyr_udc0: &usbotg_fs {
      pinctrl-0 = <&usb_otg_fs_dm_pa11 &usb_otg_fs_dp_pa12
                 &usb_otg_fs_id_pa10>;
+     pinctrl-names = "default";
      status = "okay";
   };

So error is caused by a change in the devicetree-overlay.
Do you have a idea how to solve this?

@pablogs9
Copy link
Member

Can you update the PR?

@scrapforge
Copy link
Contributor Author

I'm sorry, but i dont understand you. Could you explain it?

@pablogs9
Copy link
Member

Sure, I wonder if you can update the PR with the required changes in order to make the CI pass.

@scrapforge
Copy link
Contributor Author

I added in prj.conf a CONFIG_RING_BUFFER=y line, due to an error while building, which stated an undefined reference occured, similar to this on zephyrproject-rtos/zephyr#30183.

Could you provide details on how to trigger this build error?

Steps to trigger this error:

results in the error message:

.../zephyrproject/applications/micro_ros_zephyr_module/modules/libmicroros/microros_transports/serial/microros_transports.c:44: undefined reference to `ring_buf_put'
.../zephyrproject/applications/micro_ros_zephyr_module/modules/libmicroros/microros_transports/serial/microros_transports.c:102: undefined reference to `ring_buf_get'

prj.conf Outdated Show resolved Hide resolved
app.overlay Outdated Show resolved Hide resolved
app.overlay Show resolved Hide resolved
app.overlay Show resolved Hide resolved
prj.conf Outdated Show resolved Hide resolved
@pablogs9
Copy link
Member

#126 merged, could you rebase?

@scrapforge
Copy link
Contributor Author

#126 merged, could you rebase?

I hope i did it right.

@scrapforge
Copy link
Contributor Author

scrapforge commented Oct 24, 2023

The problem that I ran (originally) into has been, that my code did compile, but when i flashed it, it didn't work. It took me while to understand, that the problem was caused by this line

device_get_binding("CDC_ACM_0");

This function call itself it not dependent on any correct hardware configuration. A string is a string.

But at runtime it tried to get the device, by a label/name not known to the zephyr OS, due to lacking configuration. So the surrounding RCCHECK of the function, which calls this code line indirect, did throw an error:

RCCHECK(rclc_support_init(&support, 0, NULL, &allocator));

So i dont think that any CI, which only compiles the code (as far as i unterstand the log), would actually detect my problem, because it only shows up at runtime.

So i can currently confirm my solution for my setup only
(Zephyr 3.5, Zephyr-Sdk 0.16, Board nucleo_h743zi)

@scrapforge
Copy link
Contributor Author

I> Is it possible to embedded this dependency of the USB CDC transport at KConfig level. Something like the following in modules/libmicroros/KConfig

It is possible, also all warnings to the use of "select" in Kconfig apply.
The only other option i found, would to use set(<TRANSPORT_VARIABLE> 1 ) in CMake and use this to conditional add .conf files. But this couldn't be managed by Kconfig anymore, because this has to be done before Zephyr is included.

Copy link
Member

@pablogs9 pablogs9 left a comment

Choose a reason for hiding this comment

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

Last changes and we can merge

prj.conf Outdated Show resolved Hide resolved
modules/libmicroros/Kconfig Outdated Show resolved Hide resolved
scrapforge and others added 2 commits October 26, 2023 08:32
Co-authored-by: Pablo Garrido <[email protected]>
@pablogs9
Copy link
Member

@mergify backport humble rolling

@mergify
Copy link
Contributor

mergify bot commented Oct 26, 2023

backport humble rolling

✅ Backports have been created

@pablogs9 pablogs9 merged commit 7c5edcd into micro-ROS:iron Oct 26, 2023
3 checks passed
mergify bot pushed a commit that referenced this pull request Oct 26, 2023
* Added Ringbuffer-config in prj.conf for eliminating error

* Added Label CDC_ACM_0 label with an overlay so "modules/libmicroros/microros_transports/serial-usb/microros_transports.c" can find the device

* Update app.overlay

* Update app.overlay

A fix for backwards compability

* Update app.overlay

* Update app.overlay

* Update prj.conf

* Better comments

* Changed settings to conditional from transport choice

* Update prj.conf

Co-authored-by: Pablo Garrido <[email protected]>

* Update modules/libmicroros/Kconfig

Co-authored-by: Pablo Garrido <[email protected]>

---------

Co-authored-by: scrapforge <[email protected]>
Co-authored-by: Pablo Garrido <[email protected]>
(cherry picked from commit 7c5edcd)

# Conflicts:
#	prj.conf
mergify bot pushed a commit that referenced this pull request Oct 26, 2023
* Added Ringbuffer-config in prj.conf for eliminating error

* Added Label CDC_ACM_0 label with an overlay so "modules/libmicroros/microros_transports/serial-usb/microros_transports.c" can find the device

* Update app.overlay

* Update app.overlay

A fix for backwards compability

* Update app.overlay

* Update app.overlay

* Update prj.conf

* Better comments

* Changed settings to conditional from transport choice

* Update prj.conf

Co-authored-by: Pablo Garrido <[email protected]>

* Update modules/libmicroros/Kconfig

Co-authored-by: Pablo Garrido <[email protected]>

---------

Co-authored-by: scrapforge <[email protected]>
Co-authored-by: Pablo Garrido <[email protected]>
(cherry picked from commit 7c5edcd)
pablogs9 added a commit that referenced this pull request Oct 26, 2023
) (#129)

* Fix of unknown Ringbuffer and not found USB-Serial Device (#125)

* Added Ringbuffer-config in prj.conf for eliminating error

* Added Label CDC_ACM_0 label with an overlay so "modules/libmicroros/microros_transports/serial-usb/microros_transports.c" can find the device

* Update app.overlay

* Update app.overlay

A fix for backwards compability

* Update app.overlay

* Update app.overlay

* Update prj.conf

* Better comments

* Changed settings to conditional from transport choice

* Update prj.conf

Co-authored-by: Pablo Garrido <[email protected]>

* Update modules/libmicroros/Kconfig

Co-authored-by: Pablo Garrido <[email protected]>

---------

Co-authored-by: scrapforge <[email protected]>
Co-authored-by: Pablo Garrido <[email protected]>
(cherry picked from commit 7c5edcd)

# Conflicts:
#	prj.conf

* Update prj.conf

---------

Co-authored-by: scrapforge <[email protected]>
Co-authored-by: Pablo Garrido <[email protected]>
pablogs9 pushed a commit that referenced this pull request Oct 26, 2023
* Added Ringbuffer-config in prj.conf for eliminating error

* Added Label CDC_ACM_0 label with an overlay so "modules/libmicroros/microros_transports/serial-usb/microros_transports.c" can find the device

* Update app.overlay

* Update app.overlay

A fix for backwards compability

* Update app.overlay

* Update app.overlay

* Update prj.conf

* Better comments

* Changed settings to conditional from transport choice

* Update prj.conf

Co-authored-by: Pablo Garrido <[email protected]>

* Update modules/libmicroros/Kconfig

Co-authored-by: Pablo Garrido <[email protected]>

---------

Co-authored-by: scrapforge <[email protected]>
Co-authored-by: Pablo Garrido <[email protected]>
(cherry picked from commit 7c5edcd)

Co-authored-by: scrapforge <[email protected]>
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