Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix msom usb + ble setup names #2732

Closed
wants to merge 1 commit into from

Conversation

scott-brust
Copy link
Member

Problem

Msom setup shows the wrong name: Mseries. It should be M SoM
Same for P2 BLE setup.

Solution

Fix the hardcoded USB + BLE setup strings

Steps to Test

Build + flash branch. Setup msom on setup.particle.io, name should show as M SoM

Example App

tinker

References

Links to the Community, Docs, Other Issues, etc..


Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

#endif // defined(MODULE_FUNCTION) && MODULE_FUNCTION != 2 // MOD_FUNC_BOOTLOADER

#define PRODUCT_SERIES "Mseries"
#define PRODUCT_SERIES "M SoM"
Copy link
Contributor

Choose a reason for hiding this comment

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

This/these were specifically chosen with the "Series" text. Are we breaking convention?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nick specifically requested this change here

Copy link
Member

Choose a reason for hiding this comment

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

Changing from Series to some other convention is OK.

My main concern is the space. This constant is used for the BLE advertising name BleGap::setDeviceName and the Wi-Fi setup SSID fetch_or_generate_setup_ssid (is that still a thing in modern devices?). I'm not sure what issues we introduce with BLE and Wi-Fi stacks if we add a space in the name.

I think it would be safer to define PRODUCT_SERIES as MSoM.

@@ -81,7 +81,7 @@
#endif // PLATFORM_ID != PLATFORM_BORON

#if PLATFORM_ID == PLATFORM_ESOMX
#define PRODUCT_SERIES "Eseries"
#define PRODUCT_SERIES "E SoM X"
Copy link
Member

Choose a reason for hiding this comment

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

Likewise ESoMX would be safer for PRODUCT_SERIES

@mrlambchop
Copy link
Contributor

spaces in ble and wifi are fine - we need to be consistent in our product messaging mostly (so I'm assuming these follow the existing B SoM definitions?)

@eberseth
Copy link
Contributor

The PR changes the product reported in USB enumeration while the USB list command remains "M SoM". I am assuming setup takes the system USB enumeration string and the CLI sanitizes this already to "M SoM".

As reported before running this branch:

$ particle usb list
.............. [........................] (M SoM)

$ dmesg
[15713.215453] usb 1-1: new high-speed USB device number 4 using xhci_hcd
[15713.849042] usb 1-1: config 1 interface 2 altsetting 0 bulk endpoint 0x2 has invalid maxpacket 64
[15713.849048] usb 1-1: config 1 interface 2 altsetting 0 bulk endpoint 0x83 has invalid maxpacket 64
[15713.857880] usb 1-1: New USB device found, idVendor=2b04, idProduct=c023, bcdDevice= 2.51
[15713.857886] usb 1-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[15713.857889] usb 1-1: Product: Mseries
[15713.857892] usb 1-1: Manufacturer: Particle
[15713.857895] usb 1-1: SerialNumber: ........................
[15713.861620] cdc_acm 1-1:1.1: ttyACM0: USB ACM device

After running this branch:

$ particle usb list
.............. [........................] (M SoM)

$ dmesg
[16432.209940] usb 1-1: new high-speed USB device number 10 using xhci_hcd
[16432.359128] usb 1-1: config 1 interface 2 altsetting 0 bulk endpoint 0x2 has invalid maxpacket 64
[16432.359133] usb 1-1: config 1 interface 2 altsetting 0 bulk endpoint 0x83 has invalid maxpacket 64
[16432.367417] usb 1-1: New USB device found, idVendor=2b04, idProduct=c023, bcdDevice= 2.51
[16432.367422] usb 1-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[16432.367425] usb 1-1: Product: M SoM
[16432.367429] usb 1-1: Manufacturer: Particle
[16432.367431] usb 1-1: SerialNumber: ........................
[16432.370733] cdc_acm 1-1:1.1: ttyACM0: USB ACM device

@scott-brust
Copy link
Member Author

My main concern is the space. This constant is used for the BLE advertising name BleGap::setDeviceName and the Wi-Fi setup SSID fetch_or_generate_setup_ssid (is that still a thing in modern devices?). I'm not sure what issues we introduce with BLE and Wi-Fi stacks if we add a space in the name.

The only limitation is that the string must fit in the 20 character max for BLE advertising name. This includes a trailing serial number added by default. I tested this PR and can see the string M SoM-XXXXXX is advertised correctly so we should be under the limit.
IMG_0259

spaces in ble and wifi are fine - we need to be consistent in our product messaging mostly (so I'm assuming these follow the existing B SoM definitions?)

Bsom advertises as Boron-XXXXX
IMG_0260

It reports B SoM CDC Mode in usb:

lsusb
...
Bus 020 Device 007: ID 2b04:c017 2b04 B SoM CDC Mode  Serial: e00fce68cfe73d72b2773776

@mrlambchop Do we want to change the bsom USB and BLE names as well?

@scott-brust scott-brust force-pushed the fix/platform-product-series-names branch from 4089a4e to d03ae6f Compare February 1, 2024 21:58
@avtolstoy
Copy link
Member

Superseded by #2753

@avtolstoy avtolstoy closed this Mar 14, 2024
@avtolstoy avtolstoy deleted the fix/platform-product-series-names branch March 14, 2024 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants