Add usbtmc class driver (READY FOR TESTING) #160

pigrew · 2019-09-14T17:46:19Z

Here is an initial revision of a USBTMC class driver.

It has most of the features I need, but not all of the features that exist in USBTMC. I added some comments on what's lacking in the headers of src/class/usbtmc/usbtmc_device.c. It's missing many of the "required" specifications, but is implemented enough to meet most needs.

The example application does not have any message/passing or multi-thread capability at the moment. I'm not planning to use a usbtmc task in the example, but I will in my more final project.

I don't intend on this being accepted right away, as I'm likely to create some breaking changes in the interface, but I want to get feedback on what I have now.

Note that it's against the "develop" branch because it requires the class-type EP-recipient patch to work.

This has been tested with NI MAX's visa control panel on Windows 10. Send, receive, query, and read STB all work (or at least with very short messages which won't overflow buffers).

(I fear I'm created this PR too soon; can delete if I'm committing to it too much, or can just start pushing once a day or something. I also understand why I see so many incomplete USBTMC implementations in commercial hardware.)

Some other implementations:

karlp/discotmc
FTDI FT51A
Linux USBTMC host class

examples/device/usbtmc/src/usb_descriptors.c

src/device/usbd.c

pigrew · 2019-09-14T19:42:02Z

src/class/usbtmc/usbtmc_device.c

+      case USBTMC_MSGID_VENDOR_SPECIFIC_IN:
+      case USBTMC_MSGID_USB488_TRIGGER:
+      default:
+        TU_VERIFY(false);


To NAK (stall?), should it TU_VERIFY(false) or return false?

…t USB indicator pulse

…rts makes sense. Probably adding too many concurance issues; need to figure out semaphores.

src/class/usbtmc/usbtmc.h

ladyada · 2019-09-15T22:56:20Z

neat! ive never heard of this

pigrew · 2019-09-16T03:05:44Z

neat! ive never heard of this

@ladyada: Yeah, it's a quite powerful protocol. Most "test instruments" like DMM or oscilloscopes speak it over USB. It's designed to replace GPIB (and have all of its features, where ever possible). I've been wanting to use it it in personal projects for a while, but have not found any complete open-source implementations. My feeling is that each company (Keysight, Tek, LeCroy, etc likely have their own implementations, and these days all of those would be on an embedded Linux or Windows SoC).

This USBTMC spec is much more complicated to implement than I had anticipated. My code in this request (at the moment) is a bit too simplistic (and the number of lines will probably double or triple :/). I've finished up implemented the "indicator pulse" LED flash, but the transaction aborts and device clears are needing a lot of planning (due to the asynchronous nature of them).

I'm still aiming to have minimal locking, and such. I don't want it to implement its own task, so some locking will be necessary as it can be called both from the USBD core and application code.

I also ran into a few implementation problems with my stm32 fsdev driver which I'm also fixing (it didn't properly support endpoints != 64 bytes, and had a typo-bugs, and had 400 B of type conversions going into its flash). Those should be put together into a PR early this week.

I'll update this PR in a day or once I figure out how to cope with concurrency and aborting transactions. Some progress (both USBTMC and stm fsdev driver) are my fork.

hathach

Thank you very much for yet another great PR. It is never be too soon to submit PR, you can constantly push the new code to update the PR. Even after merged, I did constantly changing the stack all around.

There is only API naming changes to keep thing consistency for now, though I will read some docs and try to get familiar with this class later on. Meanwhile, would you mind give me a bit of instruction on how to test this class: e.g application link, or simple instructions to use those app to test with.

examples/device/usbtmc/src/tusb_config.h

examples/device/usbtmc/src/usbtmc_app.c

src/class/usbtmc/usbtmc_device.h

src/device/usbd.c

src/class/usbtmc/usbtmc_device.h

hathach · 2019-09-16T05:41:44Z

src/class/usbtmc/usbtmc.h

+};
+
+typedef enum {
+  USBTMC_bREQUEST_INITIATE_ABORT_BULK_OUT      = 1u,


all UPPER case, you can remove the b

src/class/usbtmc/usbtmc.h

pigrew · 2019-09-16T12:51:45Z

Thank you very much for yet another great PR. It is never be too soon to submit PR, you can constantly push the new code to update the PR. Even after merged, I did constantly changing the stack all around.

There is only API naming changes to keep thing consistency for now, though I will read some docs and try to get familiar with this class later on. Meanwhile, would you mind give me a bit of instruction on how to test this class: e.g application link, or simple instructions to use those app to test with.

Yes, I'l l do that. Which operating system do you use? The drivers are different for different OS. I can add a simple python script to talk to the device once the drivers are installed.

…ir own lines.

…m... oops.

frequency.

…ng bulk in)

pigrew · 2019-09-22T02:01:55Z

@hathach, With the latest changes, I think the class is ready for testing. The main functionality is working well. I'm worried a little bit about concurrency (I added locking for the state machine variable, but it isn't sufficient).

I added a python test script to the examples folder, which demonstrates the core functionality and some of the error handling.

I'm happy for it to be merged, or sit here in testing for a while. I'm sure that there will be issues that will come up.

pigrew · 2019-09-23T03:56:01Z

I seem to have forgotten an important feature. The app layer has to be able to signal that it's busy, and have the class driver NAK the bulk-out endpoint. This will require another function for the app layer to call when it's ready. Delay state are not mentioned in the specification, so I think that a NAK is the proper response when the micro's internal buffers are full.

…id. Assume a single interface, for now.

pigrew · 2019-09-24T17:44:34Z

Having the app control the interface flow control is implemented. Also, I removed references to rhport from the application layer, as I think this only applies to host drivers? If anything, interface_id should be added, but for now I'm assuming a single interface.

My python regression testing script is still passing, though only tested with a 64 byte endpoint size on STM32F0.

hathach

Fantastic works on this PR. There is only minor naming convention for the API. If you don't have time, I will merge this first and then rename it later on. Not a big deal at all.

docs/concurrency.md

hathach · 2019-09-25T09:33:51Z

src/class/usbtmc/usbtmc_device.h

+// * (successful) tud_usbtmc_app_check_abort_bulk_in_cb
+// * (successful) usmtmcd_app_bulkOut_clearFeature_cb
+
+void tud_usbtmc_app_open_cb(uint8_t interface_id);


should we replace tud_usbtmc_app by simply tud_usbtmc_ dropping *app in API

I'm not sure. I've found it hard to keep track of which functions are for which direction of communication, and keeping app there makes it more clear that certain functions are for implementation by the app, not the class library.

I think other libraries fix this by using structs of function pointers to create a namespace, but the TUSB code at the moment doesn't do that.

I can go either way with this question.

It is rather simple, if the API is invoked by the stack and application have to implement that function, it is a callback and must end with _cb() . Otherwise, it doesn't has _cb() e.g

tud_cdc_write() : implemented by stack
tud_cdc_rx_cb(): implemented by application

I'm going to change this to be as you suggested.

I usually read left-to-right, so looking for the _cb is less easy, as I need to read left to right to center.

However, there are more levels of complexity....

app calls class (tud_class_*)

class calls app (tud_class_*_cb)

stack calls class (tud_class_* or tud_class_*_cb); // Sometimes with _cb, other times without _cb?

class calls stack (usbd_*)

Number 1 and 2 and 3 have overlap. This somewhat impacts the app developer since auto-complete will suggest both.

src/class/usbtmc/usbtmc_device.h

examples/device/usbtmc/src/tusb_config.h

… extern struct.

pigrew · 2019-09-25T16:53:49Z

@hathach Would you mind taking a look at the latest changes?

They are mostly cosmetic, but I did have some small updates to the testing script, and also changed the get_capabilities method to use a callback function instead of an extern struct.

hathach

Perfect !!! Thank you very much for putting lots of effort into this PR.

Add usbtmc class driver.

1cae969

pigrew commented Sep 14, 2019

View reviewed changes

examples/device/usbtmc/src/usb_descriptors.c Show resolved Hide resolved

src/device/usbd.c Outdated Show resolved Hide resolved

src/device/usbd.c Show resolved Hide resolved

pigrew commented Sep 14, 2019

View reviewed changes

pigrew added 4 commits September 14, 2019 16:51

Combined status response enum (std with 488), support trigger.

e74c6b0

(and update the example to match)

acd8fa7

Handle USBTMC TermChar.

6e00886

Change requset type parameter back to uint8_t (fix type cast), suppor…

64bfec7

…t USB indicator pulse

pigrew force-pushed the usbtmc branch from 9efa9a5 to 64bfec7 Compare September 14, 2019 22:11

Simulate query delay; getting to a point where implementing clear/abo…

e53e9bd

…rts makes sense. Probably adding too many concurance issues; need to figure out semaphores.

pigrew commented Sep 15, 2019

View reviewed changes

src/class/usbtmc/usbtmc.h Outdated Show resolved Hide resolved

hathach requested changes Sep 16, 2019

View reviewed changes

pigrew changed the title ~~Add usbtmc class driver.~~ Add usbtmc class driver. (IN DEVELOPMENT) Sep 16, 2019

pigrew changed the title ~~Add usbtmc class driver. (IN DEVELOPMENT)~~ Add usbtmc class driver (IN DEVELOPMENT) Sep 16, 2019

pigrew and others added 14 commits September 17, 2019 13:29

Clear

c755aee

More proper handling of clear.

e234998

Changes1...

4587100

changes 2

91cd247

change 3

c7006a2

Correct port assigned on the STM32F070 BSP

45b69a3

Update boards.md

44013e6

Update boards.md

fd4be23

TI compiler quirks, and le byte swapping functions.

a0b6b81

Massive copy&paste typo of mine in the 32-bit byte swapping function....

41e8b87

Off-by-one error in Synopsys assertions. (and move some braces to the…

8d05b70

…ir own lines.

This demonstrates that I don't pre-build things before committing the…

1ceeab2

…m... oops.

Add pyboard (v1.1) BSP. cdc_msc_hid example tested.

192e1de

pyboardv11: Fix typo in HSE_VALUE define so that SysTick runs at correct

ff36957

frequency.

pigrew added 11 commits September 19, 2019 19:00

Add trigger test code.

0f14351

Add testcase for aborting bulk in with no data queued (and fix aborti…

3464431

…ng bulk in)

Move descriptors to usbd.h

7e0490b

s/USBTMC488/USB488/ ; fix types in bitfield

d742c3e

Rename USBTMC_CFG_ENABLE_488, and fix build with it off.

f3d6d01

Rename functions to include tud_ and _cb

2a7ca15

(little typo fix)

02b468f

Merge remote-tracking branch 'origin/master' into usbtmc

345806a

Merge branch 'master' into usbtmc

ec3414c

Merge branch 'clear_feature_ordering' into usbtmc

2aa10da

Lots of updates (especially error handling)

fa5b5e4

pigrew changed the title ~~Add usbtmc class driver (IN DEVELOPMENT)~~ Add usbtmc class driver (READY FOR TESTING) Sep 22, 2019

pigrew added 2 commits September 22, 2019 09:30

Merge remote-tracking branch 'origin/master' into usbtmc

757c2cd

Retool stalling EP on xfer errors.

e61f2b7

pigrew added 2 commits September 24, 2019 13:11

Let application control interface latency...

1010555

Remove rhport from app code. If anything, it should be the interface_…

624f4bf

…id. Assume a single interface, for now.

I never understood why I need to write void as empty function arguments.

412a279

hathach reviewed Sep 25, 2019

View reviewed changes

pigrew added 4 commits September 25, 2019 12:46

Rename USBTMC functions.

2d4352b

Change getting the capabilities structure to a callback instead of an…

3a87dd6

… extern struct.

USBTMC: Minor updates to testing script.

6c6a01c

Remove _app from callback function names.

6841b23

hathach approved these changes Sep 25, 2019

View reviewed changes

hathach merged commit 9a8d23e into hathach:master Sep 25, 2019

pigrew deleted the usbtmc branch September 25, 2019 20:41

pigrew mentioned this pull request Sep 27, 2019

Add USBTMC class device implementation #129

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add usbtmc class driver (READY FOR TESTING) #160

Add usbtmc class driver (READY FOR TESTING) #160

pigrew commented Sep 14, 2019 •

edited

Loading

pigrew Sep 14, 2019 •

edited

Loading

ladyada commented Sep 15, 2019

pigrew commented Sep 16, 2019

hathach left a comment •

edited

Loading

hathach Sep 16, 2019

pigrew commented Sep 16, 2019

pigrew commented Sep 22, 2019 •

edited

Loading

pigrew commented Sep 23, 2019

pigrew commented Sep 24, 2019 •

edited

Loading

hathach left a comment

hathach Sep 25, 2019

pigrew Sep 25, 2019

hathach Sep 25, 2019 •

edited

Loading

pigrew Sep 25, 2019

pigrew commented Sep 25, 2019

hathach left a comment

Add usbtmc class driver (READY FOR TESTING) #160

Add usbtmc class driver (READY FOR TESTING) #160

Conversation

pigrew commented Sep 14, 2019 • edited Loading

pigrew Sep 14, 2019 • edited Loading

Choose a reason for hiding this comment

ladyada commented Sep 15, 2019

pigrew commented Sep 16, 2019

hathach left a comment • edited Loading

Choose a reason for hiding this comment

hathach Sep 16, 2019

Choose a reason for hiding this comment

pigrew commented Sep 16, 2019

pigrew commented Sep 22, 2019 • edited Loading

pigrew commented Sep 23, 2019

pigrew commented Sep 24, 2019 • edited Loading

hathach left a comment

Choose a reason for hiding this comment

hathach Sep 25, 2019

Choose a reason for hiding this comment

pigrew Sep 25, 2019

Choose a reason for hiding this comment

hathach Sep 25, 2019 • edited Loading

Choose a reason for hiding this comment

pigrew Sep 25, 2019

Choose a reason for hiding this comment

pigrew commented Sep 25, 2019

hathach left a comment

Choose a reason for hiding this comment

pigrew commented Sep 14, 2019 •

edited

Loading

pigrew Sep 14, 2019 •

edited

Loading

hathach left a comment •

edited

Loading

pigrew commented Sep 22, 2019 •

edited

Loading

pigrew commented Sep 24, 2019 •

edited

Loading

hathach Sep 25, 2019 •

edited

Loading