klipper icon indicating copy to clipboard operation
klipper copied to clipboard

usbcan: support `ip link set can0 type can bitrate $bitrate`

Open ayufan opened this issue 9 months ago • 5 comments

Makes Klipper to be a quite functional usb-to-can with externally controllable bitrate.

Quite handy, as the bitrate is now configured by the system.

This was/is tested, and running for a long time on:

  • Was tested on various rp2040 boards
  • Was tested on various stm32 boards (BTT Octopus 1.1, but also Cheetah v2 and v3)
  • It aligns code to follow the same codestyle for atsam and atsamd boards

The change might create incompatibility for some users: people might have different can frequency configured in FW vs one for network interface on a linux system. Up until now the system requested bitrate was ignored.

ayufan avatar Mar 21 '25 17:03 ayufan

Thanks for working on this. I agree it would be useful to set the "usb to canbus bridge" frequency from Linux instead of from "make menuconfig".

This change does have a risk of introducing bugs and I think in order to merge it would be needed to break up the change into multiple self-standing commits that could be reverted if we run into a problem. This is particularly important for changes across multiple micro-controller architectures as the risk of introducing a regression is high. A possible order of commits may look something like: a commit that introduces empty canhw_start() and canhw_stop() functions that are called from canserial.c; a series of commits, one per architecture, converting each architecture to actually support canhw_start(); a commit that adds support for callbacks to usb_do_xfer(); a commit that adds support for setting the canbus bridge frequency from Linux.

Also, if the frequency is set from Linux, I think "make menuconfig" should not prompt for a frequency. (As asking for a frequency that is not actually used would be confusing.) Similarly, the documentation would need to be updated to reflect the new support.

Cheers, -Kevin

KevinOConnor avatar Apr 03 '25 18:04 KevinOConnor

Thanks @KevinOConnor I will make those changes and get back to you. It will take me a few weeks due to upcoming travel.

ayufan avatar Apr 03 '25 18:04 ayufan

Also, if the frequency is set from Linux, I think "make menuconfig" should not prompt for a frequency. (As asking for a frequency that is not actually used would be confusing.) Similarly, the documentation would need to be updated to reflect the new support.

I see two cases:

  • can device: we need to set frequency up-front
  • usbcan device: we need some default, as setting can frequency is likely not a requirement (will check this)

ayufan avatar Apr 03 '25 18:04 ayufan

Awesome work. Having the bridge ignore the set frequency was really confusing to me at first. Especially since my printer* didn't even have the MCU in the out of the box config.

This will also make verifying if a certain speed works much easier.

* Magneto X

EmperorArthur avatar Apr 07 '25 23:04 EmperorArthur

Thank you for your contribution to Klipper. Unfortunately, a reviewer has not assigned themselves to this GitHub Pull Request. All Pull Requests are reviewed before merging, and a reviewer will need to volunteer. Further information is available at: https://www.klipper3d.org/CONTRIBUTING.html

There are some steps that you can take now:

  1. Perform a self-review of your Pull Request by following the steps at: https://www.klipper3d.org/CONTRIBUTING.html#what-to-expect-in-a-review If you have completed a self-review, be sure to state the results of that self-review explicitly in the Pull Request comments. A reviewer is more likely to participate if the bulk of a review has already been completed.
  2. Consider opening a topic on the Klipper Discourse server to discuss this work. The Discourse server is a good place to discuss development ideas and to engage users interested in testing. Reviewers are more likely to prioritize Pull Requests with an active community of users.
  3. Consider helping out reviewers by reviewing other Klipper Pull Requests. Taking the time to perform a careful and detailed review of others work is appreciated. Regular contributors are more likely to prioritize the contributions of other regular contributors.

Unfortunately, if a reviewer does not assign themselves to this GitHub Pull Request then it will be automatically closed. If this happens, then it is a good idea to move further discussion to the Klipper Discourse server. Reviewers can reach out on that forum to let you know if they are interested and when they are available.

Best regards, ~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

github-actions[bot] avatar Apr 22 '25 00:04 github-actions[bot]