Introduced new test suite for PWM protocol- basic test by CPPavithra · Pull Request #90 · beagleboard/greybus-zephyr

CPPavithra · 2026-01-29T19:56:35Z

This PR adds the initial integration test infrastructure for the PWM protocol.
-> Uses native_sim board target.
-> Uses the vnd,pwm (Vendor PWM) driver instead of pwm_fake to avoid linker dependency issues with the FFF library on Zephyr 4.2.0.
-> Validates the control protocol version handshake.
-> Validates that the Greybus stack correctly routes GB_PWM_TYPE_PWM_COUNT to the driver and returns a response on the correct CPort.
-> Target: native_sim
-> Command: west build -p always -b native_sim tests/greybus/integration/pwm -t run
-> Result: PROJECT EXECUTION SUCCESSFUL

Ayush1325 · 2026-02-02T06:56:55Z

tests/greybus/integration/pwm/src/main.c

+	if (resp.msg == NULL) {
+		zassert_not_null(resp.msg, "Greybus stack did not respond on CPort %d", pwm_cport);
+	}
+	zassert_equal(resp.cport, pwm_cport, "Response received on wrong CPort");


This test doesn't really test anything. Should check the response.

Fixed. I have updated the test to explicitly verify the payload.

Note: The vnd,pwm driver on native_sim reports 0 channels by default. I added an assertion for count >= 0 and GB_OP_SUCCESS. This proves the stack correctly routed the request and packed the response structure, satisfying the integration test requirements.

tests/greybus/integration/pwm/src/main.c

Ayush1325 · 2026-02-02T06:59:58Z

tests/greybus/integration/pwm/testcase.yaml

    integration_platforms:
      - native_sim
-    tags: test_framework
+    tags: greybus pwm


Why these random tags?

Fixed. I've standardized the tags to just greybus. I must have missed that from my testing setup.

tests/greybus/integration/pwm/src/main.c

Ayush1325 · 2026-02-06T07:49:27Z

tests/greybus/integration/pwm/src/main.c

+
+	zassert_equal(resp.cport, pwm_cport, "Response received on wrong CPort");
+
+	zassert_equal(resp.msg->header.type, GB_PWM_TYPE_PWM_COUNT | GB_TYPE_RESPONSE_FLAG,


There is a helper macro GB_RESPONSE. Use that.

Ayush1325 · 2026-02-06T07:49:53Z

tests/greybus/integration/pwm/src/main.c

+
+	/*The vnd,pwm driver on native_sim might report 0 channels depending on the overlay
+	 * configuration. We verify that the value is readable (>=0).*/
+	printk("PWM Driver reported %d channels\n", pwm_resp->count);


Better to use Logs rather than normal print

tests/greybus/integration/pwm/src/main.c

Ayush1325 · 2026-02-07T16:00:52Z

tests/greybus/integration/pwm/src/main.c

+#include <zephyr/sys/byteorder.h>
+#include <zephyr/logging/log.h>
+
+LOG_MODULE_REGISTER(greybus_pwm_test, LOG_LEVEL_INF);


I would just use the CONFIG_GREYBUS_LOG_LEVEL instead of hardcoding to info.

Ayush1325 · 2026-02-07T16:02:14Z

tests/greybus/integration/pwm/src/main.c

+	resp = gb_transport_get_message();
+	zassert_not_null(resp.msg, "No version response received");
+
+	zassert_equal(resp.msg->header.result, GB_OP_SUCCESS, "Version handshake failed");


Use gb_message_is_success helper function that already exists.

Ayush1325 · 2026-02-07T16:03:08Z

tests/greybus/integration/pwm/src/main.c

+	zassert_equal(resp.msg->header.result, GB_OP_SUCCESS, "Operation failed with error: %d",
+		      resp.msg->header.result);
+
+	struct gb_pwm_count_response *pwm_resp = (struct gb_pwm_count_response *)resp.msg->payload;


Probably make this a const. Also, I prefer forward declaration.

Ayush1325 · 2026-02-07T16:03:47Z

tests/greybus/integration/pwm/src/main.c

+	zassert_not_null(resp.msg, "No enable response received");
+	zassert_equal(resp.msg->header.type, GB_RESPONSE(GB_PWM_TYPE_ENABLE), "Wrong Type");
+
+	if (resp.msg->header.result != GB_OP_SUCCESS) {


No need. Just make anything you want to print part of zassert.

I have updated the assertions as requested (removed the conditional logic).

CI Status- The enable and disable tests are now showing as Failed (Red) in the CI.
This is as per expected. The test is correctly catching that the driver returns error 254 because ofIssue #55(Driver reports 0 channels).
This confirms the test suite is working and successfully reproducing the driver issue.

The rest of the feedback has also been implemented.

No, the curret issue is that driver will report 1 channel, not 0 (

greybus-zephyr/subsys/greybus/greybus_cport.c

Line 46 in 8a1714d

static struct gb_pwm_channel_data gb_pwm_channel_data_arr_##_idx[1]; \

). This might be some other bug.

No, the curret issue is that driver will report 1 channel, not 0 (

greybus-zephyr/subsys/greybus/greybus_cport.c

Line 46 in 8a1714d

static struct gb_pwm_channel_data gb_pwm_channel_data_arr_##_idx[1]; \

). This might be some other bug.

Oh, I see, so i think this definitely points to a functional bug in the simulation backend or binding.
Since this test is correctly catching that bug, CI is now failing (red), which prevents merging.

So how would you prefer I handle this?
-> Should I mark the enable/disable tests as ZTEST_SKIP() for now (with a comment linking to the bug) so we can merge the test infrastructure?
-> Or should I leave this PR open and try to debug/fix the native_sim driver issue as part of this PR?

You fix the issue as part of this PR, just a separate commit

I just tried using debug logs in the pwm-protocols, and running the test again for much better clarity. It returns -95 (EOPNOTSUPP) (when i cross-referenced it with the docs)
I think this confirms that the existing native_sim driver I used (vnd,pwm) does not fully support enable/disable operations requested.

Since vnd,pwm is part of the core Zephyr tree (not this repo), fixing the driver would require an upstream PR to Zephyr, which is outside the scope of this PR. I can probably dive deeper into the issue and I can open a follow-up PR later to add a custom Mock PWM driver if we really need full coverage.

Doing a bit of grepping, I think you want to use zephyr,fake-pwm instead of vnd,pwm.

Doing a bit of grepping, I think you want to use zephyr,fake-pwm instead of vnd,pwm.

The zephyr,fake-pwm edit worked :) I have implemented the fix in a separate commit as requested.
Changes:
-> Replaced vnd,pwm with the upstream zephyr,fake-pwm driver in the overlay.
-> Enabled CONFIG_PWM_FAKE and FFF to correctly mock the hardware response on native_sim.

Signed-off-by: CPPavithra <cppavithra05@gmail.com>

Ayush1325 requested changes Feb 2, 2026

View reviewed changes

CPPavithra force-pushed the edge-testcases branch 2 times, most recently from 492f9c3 to 3aa096f Compare February 5, 2026 18:58

Ayush1325 requested changes Feb 6, 2026

View reviewed changes

CPPavithra force-pushed the edge-testcases branch 2 times, most recently from a904c73 to 0b16d94 Compare February 7, 2026 10:34

Ayush1325 requested changes Feb 7, 2026

View reviewed changes

Introduced new test suite for PWM protocol- basic test

8d826c4

Signed-off-by: CPPavithra <cppavithra05@gmail.com>

CPPavithra force-pushed the edge-testcases branch from 0b16d94 to 8d826c4 Compare February 7, 2026 19:54

Changed the driver from vnd,pwm to fake-pwm to return -ENOTSUP error

f2a429a

Signed-off-by: CPPavithra <cppavithra05@gmail.com>


		zassert_equal(resp.cport, pwm_cport, "Response received on wrong CPort");

		zassert_equal(resp.msg->header.type, GB_PWM_TYPE_PWM_COUNT \| GB_TYPE_RESPONSE_FLAG,

Conversation

CPPavithra commented Jan 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ayush1325 Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Ayush1325 Feb 8, 2026 •

edited

Loading