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

Concept2 PM5 Firmware Change In April 2023 Has Broken USB Functionality #5

Open
mlyonupdesigns opened this issue Aug 12, 2023 · 0 comments

Comments

@mlyonupdesigns
Copy link

I am the developer of the PM5 firmware, and have been contacted by a user of PyRow indicating the firmware versions released on or about April 2023 no long work properly. The PM5 firmware versions affected are v34.000/173.000/212.000/257.000.

To summarize, I see in the source code (csafe_command.py) that there is logic to map messages to USB reports using length (snippet) below. The PM5 firmware change obsoleted the 63 byte report and added a 500 byte report in it's place. This change is reflected in the USB enumeration for the HID class, but since you have it hard-coded I suspect this is the source of the failure.

I think the easiest change is just to eliminate the the logic block relating to the 63 byte report and just support 21 byte and 121 byte report lengths. For your application it will have zero impact on performance. I sure your users would appreciate the fix so that they can move forward for current and future firmware versions.

Any questions, please reach out to me at mlyon@updesigns.com.

#report IDs
maxmessage = max(len(message) + 1, maxresponse)

if maxmessage <= 21:
    message.insert(0, 0x01)
    message += [0] * (21 - len(message))
elif maxmessage <= 63:
    message.insert(0, 0x04)
    message += [0] * (63 - len(message))
elif (len(message) + 1) <= 121:
    message.insert(0, 0x02)
    message += [0] * (121 - len(message))
    if maxresponse > 121:
        print "Response may be too long to recieve.  Max possible length " + str(maxresponse)
else:
    print "Message too long.  Message length " + str(len(message))
    message = []
Aeva added a commit to Aeva/PyRow that referenced this issue Apr 7, 2024
This change applies the fix recommended by @mlyonupdesigns here:
wemakewaves#5

Page 6 of the PM5 CSAFE specification indicates that the spec was revised on
04/06/23 by Mark Lyons, which is likely the change mentioned in the linked
issue:

    Changed USB report ID 4 size from 62 to 500 bytes; added some a dditonal
    enumeration definitions; V0.26

See: https://www.concept2.com/files/pdf/us/monitors/PM5_CSAFECommunicationDefinition.pdf

This change is said to be necessary for compatibility with PM5 ergs, which
happens to be what I have on hand.  I have confirmed that this commit enables
the statshow.log command to function correctly with a PM5 Concept2 RowErg.

Additional information:  I am runing this on NixOS over USB with 3.13.
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

No branches or pull requests

1 participant