Support URIx eeprom Serial Numbers #394

coolacid · 2024-08-28T19:00:03Z

When using a URIx with an eeprom, the eeprom supports setting a serial number for the device. The serial number is exposed as in the device descriptor as iSerialNumber.

This PR allows the use of a serial number to identify which device to load in chan_usbradio.

On channel hidthread startup, the PR will loop through the AST radio device strings looking for a matching configured serial number. If a match is found, the devstr is updated to match the one found in the search loop.

Should a usb device match not be found, chan_usbradio continues as normal by using the configured devstr, and finally searching for any available device.

At this time, I only have very terrible python code to modify device serial numbers, and it is not clean enough to include in this PR.

I've been using a version of this for the old apt_rpt for a while, and it's worked great. This is an attempt to port to the new V3 version.

[2024-08-28 14:37:10.221] NOTICE[11380] chan_usbradio.c: Checking for USB device with serial 1000
[2024-08-28 14:37:10.310] NOTICE[11380] chan_usbradio.c: Device Serial 1234 vs 1000
[2024-08-28 14:37:10.398] NOTICE[11380] chan_usbradio.c: Device Serial 1000 vs 1000
[2024-08-28 14:37:10.398] NOTICE[11380] chan_usbradio.c: Found device serial 1000 at 2-1:1.0 for 1999

KB4MDD · 2024-08-28T19:17:16Z

This support will also need to be added to chan_simpleusb. We are attempting to keep the same features in both drivers.

Common routines, like get_usb_serial, will need to be in res_usbradio.

Please review https://docs.asterisk.org/Historical-Documentation/Development/Historical-Policies-and-Procedures/Code-Review/Coding-Guidelines/

We will run through a code review soon.

coolacid · 2024-08-28T19:21:28Z

Thanks for the feedback. I'll see about moving the get_usb_serial to the resource. I haven't looked at simple_usb code at all, so probably need a little direction on that.

KB4MDD · 2024-08-28T20:03:54Z

You will find the routines that you changed are almost the same in simple_usb

coolacid · 2024-08-28T20:08:34Z

Thanks, a quick look seemed to say that. I'll work on implementing and testing in simple_usb a little later.

coolacid · 2024-08-28T20:42:18Z

Have to put this down for a bit. SimpleUSB works except for one bug. It's suppose to collect the serial number when an auto-assign happens. With adding some debug I see it enter my if statement just fine, but simpleusb-tune-menu won't show the serial number. If I set the serial number in the config, it all works as expected.

…rial

Allan-N · 2024-10-18T20:05:38Z

res/res_usbradio.c

+ * \retval		Length of found serial number
+ */
+
+int ast_radio_get_usb_serial(char *devstr, char *serial)


This function really needs to have the size of the "char *serial" buffer passed in as an argument.

Allan-N · 2024-10-18T20:06:20Z

res/res_usbradio.c

+
+    usb_dev = ast_radio_hid_device_init(devstr);
+    usb_handle = usb_open(usb_dev);
+    length = usb_get_string_simple(usb_handle, usb_dev->descriptor.iSerialNumber, serial, sizeof(serial));


sizeof(serial) is NOT what you want here. You want the caller to pass in the size of the provided buffer and pass that along to usb_get_string_simple()

Allan-N · 2024-10-18T20:10:49Z

channels/chan_simpleusb.c

@@ -347,6 +348,7 @@ static struct chan_simpleusb_pvt simpleusb_default = {
 	.rxondelay = 0,
 	.txoffdelay = 0,
 	.pager = PAGER_NONE,
+	.serial = "",


I'm not sure about this since the variable is "char serial[14]". Also, with the default struct being zero'd out then is this needed (kinda the same way that .devstr is not being initialized)?

Allan-N · 2024-10-18T20:12:55Z

channels/chan_simpleusb.c

+		/* If configuration has a serial number defined, find the device */
+		if (strlen(o->serial) > 0)
+		{
+			ast_log(LOG_NOTICE, "Checking for USB device with serial %s\n", o->serial);


This ast_log() ... and the others below ... really feel like debug messages. Maybe switch them to ast_debug() ?

Allan-N · 2024-10-18T20:19:59Z

channels/chan_simpleusb.c

+				/* Go through the list of usb devices, and get the serial numbers */
+				if (ast_radio_get_usb_serial(index_devstr, serial) == 0) continue;
+				ast_log(LOG_NOTICE, "Device Serial %s vs %s\n", o->serial, serial);
+				if (strcmp(o->serial, serial) == 0)


Pull the brace ('{') up to this line.

Allan-N · 2024-10-18T20:20:01Z

channels/chan_simpleusb.c

+					break;
+				}
+				/* Go through the list of usb devices, and get the serial numbers */
+				if (ast_radio_get_usb_serial(index_devstr, serial) == 0) continue;


The app_rpt project follows the asterisk coding standards where single/standalone statements are not allowed. This should be :

if (ast_radio_get_usb_serial(index_devstr, serial) == 0) { continue; }

Allan-N · 2024-10-18T20:21:09Z

channels/chan_simpleusb.c

@@ -3087,6 +3123,7 @@ static void _menu_print(int fd, struct chan_simpleusb_pvt *o)
 	ast_cli(fd, "Active radio interface is [%s]\n", simpleusb_active);
 	ast_mutex_lock(&usb_dev_lock);
 	ast_cli(fd, "Device String is %s\n", o->devstr);
+	ast_cli(fd, "Device Serial is %s\n", o->serial);


I have mixed thoughts about reporting the device serial if there is no serial number present.

coolacid · 2024-10-18T20:26:11Z

Thanks @Allan-N for the review. C isn't my primary language, so I appreciate the hand holding.

Allan-N · 2024-10-18T20:28:12Z

FYI : Asterisk Coding Guidelines

Allan-N

One other (set of) changes to consider. The tune menus allow you to switch the device associated with a node (handy/needed if you have multiple dongles connected). This is handled in chan_simpleusb.c by the usb_device_swap() function. Seems like that function also needs to be aware of the serial #.

coolacid and others added 4 commits August 28, 2024 09:26

First pass

650ae56

Fix build issues

741f765

Fix getting serial number when loading from devstr

e0bb879

Add notice when device not found, index++

5b223a7

coolacid added 2 commits August 28, 2024 15:34

Move get_usb_serial to ast_radio_get_usb_serial in res

d59c9e1

Fix bad copy/paste

dbc810d

coolacid marked this pull request as draft August 28, 2024 20:08

First simpleusb pass

5d2ebbe

Correct serial load spot, log notice when auto assigned device has se…

9e6c45d

…rial

coolacid marked this pull request as ready for review October 18, 2024 18:40

Merge branch 'master' into SerialSupport-working

bc3f8e0

Allan-N requested changes Oct 18, 2024

View reviewed changes

Allan-N assigned coolacid Oct 18, 2024

Allan-N requested review from KB4MDD, InterLinked1 and Allan-N October 18, 2024 20:34

Allan-N requested changes Oct 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support URIx eeprom Serial Numbers #394

Support URIx eeprom Serial Numbers #394

coolacid commented Aug 28, 2024

KB4MDD commented Aug 28, 2024

coolacid commented Aug 28, 2024

KB4MDD commented Aug 28, 2024

coolacid commented Aug 28, 2024

coolacid commented Aug 28, 2024

Allan-N Oct 18, 2024

Allan-N Oct 18, 2024

Allan-N Oct 18, 2024

Allan-N Oct 18, 2024

Allan-N Oct 18, 2024

Allan-N Oct 18, 2024

Allan-N Oct 18, 2024 •

edited

Loading

coolacid commented Oct 18, 2024

Allan-N commented Oct 18, 2024

Allan-N left a comment

Support URIx eeprom Serial Numbers #394

Are you sure you want to change the base?

Support URIx eeprom Serial Numbers #394

Conversation

coolacid commented Aug 28, 2024

KB4MDD commented Aug 28, 2024

coolacid commented Aug 28, 2024

KB4MDD commented Aug 28, 2024

coolacid commented Aug 28, 2024

coolacid commented Aug 28, 2024

Allan-N Oct 18, 2024

Choose a reason for hiding this comment

Allan-N Oct 18, 2024

Choose a reason for hiding this comment

Allan-N Oct 18, 2024

Choose a reason for hiding this comment

Allan-N Oct 18, 2024

Choose a reason for hiding this comment

Allan-N Oct 18, 2024

Choose a reason for hiding this comment

Allan-N Oct 18, 2024

Choose a reason for hiding this comment

Allan-N Oct 18, 2024 • edited Loading

Choose a reason for hiding this comment

coolacid commented Oct 18, 2024

Allan-N commented Oct 18, 2024

Allan-N left a comment

Choose a reason for hiding this comment

Allan-N Oct 18, 2024 •

edited

Loading