Skip to content

apps/examples: Add path option to SPI Slave test#3467

Open
eren-terzioglu wants to merge 1 commit intoapache:masterfrom
eren-terzioglu:feature/add_spislv_path_option
Open

apps/examples: Add path option to SPI Slave test#3467
eren-terzioglu wants to merge 1 commit intoapache:masterfrom
eren-terzioglu:feature/add_spislv_path_option

Conversation

@eren-terzioglu
Copy link
Copy Markdown
Contributor

Summary

  • apps/examples: Add path option to SPI Slave test

Add path option to spislv_test example

Impact

Impact on user: Yes. Users can test different SPI slave interface just changing command
Impact on build: No.
Impact on hardware: No.
Impact on documentation: No.
Impact on security: No.
Impact on compatibility: No.

Testing

esp32c6-devkitc:spislv config used.

Building

Command to build:

make -j distclean && ./tools/configure.sh esp32c6-devkitc:spislv && make -j && make download ESPTOOL_PORT=/dev/ttyUSB0

Running

spislv -p /dev/spislv0 -x 2 abba and spislv -p /dev/spislv2 -x 2 abba commands used to test

Output

Here is the output

nsh> spislv -p /dev/spislv0 -x 2 abba
Error: Failed to open /dev/spislv0: No such file or directory
nsh> spislv -p /dev/spislv2 -x 2 abba
Slave: Queuing 2 bytes for sending to master: AB BA 

simbit18
simbit18 previously approved these changes May 1, 2026
#define RX_BUFFER_SIZE 64
#define TX_BUFFER_SIZE 64
#define SOURCE_FILE "dev/spislv2" /* SPI device path */
#define DEFAULT_SPI_PATH "dev/spislv2"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@eren-terzioglu why is it spislv2 instead of spislv0 ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yup, example usage mentions printf(" spislv -p /dev/spislv0\n\n"); and 0 seems to be a defalut, please update to 0 unless other reason not to use it? :-)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Default path was spislv2. I did not break any workflow with this update. I can change example usage to spislv2 if it is fine for you? What do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, I noticed that, I just asked why spislv2? I suggest you trying it to understand why it was this way. It could be a typo in the original config

Copy link
Copy Markdown
Contributor Author

@eren-terzioglu eren-terzioglu May 4, 2026

Choose a reason for hiding this comment

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

Example creator created this to test SPI Slave for ESP32-C6. On Espressif devices we register SPI devices starting from SPI2 (because of general purpose SPI starts from SPI2) and probably he did not want to work on it much with parameter option. Only meaningfull reason comes to my mind is this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we add the default value print to the help message? :-)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added, please take a look

xiaoxiang781216
xiaoxiang781216 previously approved these changes May 2, 2026
@eren-terzioglu eren-terzioglu dismissed stale reviews from xiaoxiang781216 and simbit18 via b7ad46d May 4, 2026 07:21
@eren-terzioglu eren-terzioglu force-pushed the feature/add_spislv_path_option branch 2 times, most recently from b7ad46d to efbf644 Compare May 4, 2026 13:42
Add path option to spislv_test example

Signed-off-by: Eren Terzioglu <eren.terzioglu@espressif.com>
Copy link
Copy Markdown
Contributor

@cederom cederom left a comment

Choose a reason for hiding this comment

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

Thank you @eren-terzioglu :-)

Copy link
Copy Markdown
Contributor

@acassis acassis left a comment

Choose a reason for hiding this comment

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

@eren-terzioglu normally other applications on NuttX let the user define the device file in the menuconfig. Please change this application to do it, then the users can change it easily

@eren-terzioglu
Copy link
Copy Markdown
Contributor Author

@eren-terzioglu normally other applications on NuttX let the user define the device file in the menuconfig. Please change this application to do it, then the users can change it easily

We can add it but do we need to add for already parameterized example?

@acassis
Copy link
Copy Markdown
Contributor

acassis commented May 4, 2026

@eren-terzioglu normally other applications on NuttX let the user define the device file in the menuconfig. Please change this application to do it, then the users can change it easily

We can add it but do we need to add for already parameterized example?

I think we can use slv0 and add the default CONFIG_ to slv2 for ESP32xx board.
Probably @FelipeMdeO created this test only with ESP32 in mind, but all apps on NuttX should be generic and should work in any MCU :-)

@FelipeMdeO
Copy link
Copy Markdown
Contributor

@eren-terzioglu normally other applications on NuttX let the user define the device file in the menuconfig. Please change this application to do it, then the users can change it easily

We can add it but do we need to add for already parameterized example?

I think we can use slv0 and add the default CONFIG_ to slv2 for ESP32xx board. Probably @FelipeMdeO created this test only with ESP32 in mind, but all apps on NuttX should be generic and should work in any MCU :-)

Yes, I did this implementation and tested it only in esp32 env. Maybe I did a mistake using slv2.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants