search

jancumps / pico_scpi_usbtmc_labtool

LabVIEW compatible instrument on a Raspberry Pico
https://github.com/jancumps/pico_scpi_usbtmc_labtool/wiki
MIT License
12 stars 16 forks source link

reply when Service Request is invoked #47

Closed jancumps closed 11 months ago

jancumps commented 11 months ago

a maybe relevant discussion on NI on the payload

jancumps commented 11 months ago

Because lack of knowledge, I park this enhancement.

If restarted: this placeholder function in usbtmc_app.c is invoked if the SCPI lib thinks we need to notify: void setControlReply ()

jancumps commented 11 months ago

retrying

u77345 commented 11 months ago

In the interim, while 1. our PR to TinyUSB is created and accepted and 2. while PICO_SDK bumped up the updated TinyUSB hash we have a few options:

  1. Create a build script, to pull down all the dependencies, including libscpi and pico-sdk, into the project build folder, and apply a patch file over pico-sdk's tinyusb. Patch by git apply, which is expected to work on all target platforms.
  2. Use add_custom_target() in cmake, to patch timyusb using gnu patch. Poses a risk of patch might not be installed.
  3. Same as above, and add a GitHub CI build to build the pico binary image, taking away developer desktop dependencies on installed tools. Will be clonable and a reference for pico firmware CI build.
  4. Duplicate all necessary code to bring the srq response function up to usbtmc_app from usbtmc_device in the interim. Requires significant amount of code duplication, to recreate the types not importable from tinyUSB.
jancumps commented 11 months ago

In the interim, while 1. our PR to TinyUSB is created and accepted and 2. while PICO_SDK bumped up the updated TinyUSB hash we have a few options:

  1. Create a build script, to pull down all the dependencies, including libscpi and pico-sdk, into the project build folder, and apply a patch file over pico-sdk's tinyusb. Patch by git apply, which is expected to work on all target platforms.
  2. Use add_custom_target() in cmake, to patch timyusb using gnu patch. Poses a risk of patch might not be installed.
  3. Same as above, and add a GitHub CI build to build the pico binary image, taking away developer desktop dependencies on installed tools. Will be clonable and a reference for pico firmware CI build.
  4. Duplicate all necessary code to bring the srq response function up to usbtmc_app from usbtmc_device in the interim. Requires significant amount of code duplication, to recreate the types not importable from tinyUSB.

I created a destination branch for this exercise: 47-reply-when-service-request-is-invoked.
It contains the very last of main, develop-set-5 and feature branches that hadn't yet been merged(#58, #62, #64).

My preference for promoting a solution to the main branch, is to not touch the PicoSDK / TinyUSB sources. I'(m trying to keep the "get-involved" curve for interested parties low. While working in this dedicated branch, anything goes. We can change build script, patch, ... and tryout first

  1. get a version of the code that also supports service requests - no matter what the impact to the code base or build complexity
  2. find a way to incorporate that change in the main, without the complexity. Even if that means we have to duplicate (some) TinyUSB functionality.
u77345 commented 11 months ago

While attempting the "no TinyUSB change" approach, discovered: usbtmc_state is declared static, thus can not be extern'd from usbtmc_app.c

CFG_TUSB_MEM_SECTION static usbtmc_interface_state_t usbtmc_state

The two key fields picked up from it, rPort and ep_int_in, appears to be constant on this particular pico and always initialized to {0, 129}. However, when stubbing those out it does not send the SRQ. The patching approach on the other hand works.

The developer cost is to run a single git apply after cloning the pico-sdk

Sent PR https://github.com/jancumps/pico_scpi_usbtmc_labtool/pull/66 illustrating both. Docker build is optional.

jancumps commented 11 months ago

can not be extern'd from usbtmc_app.c

usbtmc_app.c is our own file. Or do you mean that this file can't use them.

The developer cost is to run a single git apply after cloning the pico-sdk

ok with that

Docker build is optional.

I'm not going to import docker / windows / linux specifics in the project main (even optional :) ). Same for IDE dependencies.

jancumps commented 11 months ago

... I have some conflicts importing this into the project. Before I try and pull into #47 , can you try and pull #47 changes in your branch, then commit and push your branch again.

I'll try to deal with follow up conflicts then.

Not needed - I was able to resolve

jancumps commented 11 months ago

build warning:

[build] C:/Users/jancu/Documents/git/git_pico_scpi_usbtmc/pico_scpi_usbtmc_labtool/source/usb/usbtmc_app.c:405:3: warning: implicit declaration of function 'tud_usbtmc_send_srq'; did you mean 'tud_usbtmc_open_cb'? [-Wimplicit-function-declaration]
[build]   405 |   tud_usbtmc_send_srq();
[build]       |   ^~~~~~~~~~~~~~~~~~~
[build]       |   tud_usbtmc_open_cb

build error:

source/usb/usbtmc_app.c.obj: in function `setControlReply':
[build] C:/Users/jancu/Documents/git/git_pico_scpi_usbtmc/pico_scpi_usbtmc_labtool/source/usb/usbtmc_app.c:405: undefined reference to `tud_usbtmc_send_srq'
[build] collect2.exe: error: ld returned 1 exit status
[build] ninja: build stopped: subcommand failed.
jancumps commented 11 months ago

implemented with #67