Marlin
Marlin copied to clipboard
MarlinFirmware
M485 command for RS485 support in Marlin
Description
RS485 is a bus specification, similar to a CAN bus. It uses a differential pair to transmit data. We're using Marlin for the Lumen PnP and controlling it through OpenPnP. Marlin mostly acts as a passthrough here, sending the data to the RS485 bus and listening for responses or timing out.
This PR:
- Uses the
RS485_ENABLEDline define to enable or disable this feature. - Adds the M485 command to send data to an RS485 bus.
- Accepts and verifies any responses that come back from the bus and relays them to the host.
- Uses a library to keep Marlin cleaner functionality wise.
Most of this PR is around making sure the serial port is set up correctly. Most of the logic involved is in the M485 command itself or in our library.
Requirements
This requires a board that has an RS485 transceiver such as the Opulo's motherboard for the Lumen PNP. This is to support feeders more than anything, right now. You can see more about the feeders here: https://docs.opulo.io/feeders/1-overview/feeder-overview/
Benefits
This adds the M485 command to send RS485 packets and receive responses. Currently it's hardcoded to be the Photon firmware's packet format, but the idea is for that to be changeable, we just weren't sure how best to implement it and decided against doing so until there are other examples.
One nice aspect to this is that the only thing Marlin cares about is whether the packet that goes back to the host is well formatted, matches checksums, etc. It itself does not generate any packet data. See the OpenPnP PR below for more info.
Configurations
Configurations pending PR to configurations repo.
Related Issues
OpenPnP PR: https://github.com/openpnp/openpnp/pull/1539 Photon Firmware (runs on the feeder): https://github.com/photonfirmware/photon
Everything looks good to me! I moved the new config options to the same section as the other serial ports, removed the need to define RS485_ENABLED (just looks for the serial port define), and did some basic fixes, cleanup, and optimization. I have only one burning question, posted above. I look forward to checking out your rs485 code and –if it would be useful– I can give that a quick review also.
Not sure how important this is, but I don't think this works with G-code compression in OpenPnP, which removes all spaces. When the data to send begins with a decimal digit (e.g. M485 0...), the code number is no longer parsed as 485.
I look forward to checking out your rs485 code and –if it would be useful– I can give that a quick review also.
Repo: https://github.com/Jnesselr/RS485/
I don't think this works with G-code compression in OpenPnP, which removes all spaces.
Yep. We did test it extensively, but yes if you have either "remove comments" or "compress gcode" on in OpenPnP, then it will screw this up. Remove comments is unfortunately more aggressive than just removing the comments themselves and does the same code as "compress gcode" here. They're honestly just not worth having it on, given the amount of data going through.
Also, @thinkyhead, I'm not 100% sure why it's failing. The errors I'm seeing:
/home/runner/.platformio/packages/toolchain-gccarmnoneeabi/bin/../lib/gcc/arm-none-eabi/9.2.1/../../../../arm-none-eabi/bin/ld: .pio/build/Opulo_Lumen_REV3/src/src/MarlinCore.cpp.o: in function `setup':
/home/runner/work/Marlin/Marlin/Marlin/src/MarlinCore.cpp:1666: undefined reference to `rs485_init()'
/home/runner/.platformio/packages/toolchain-gccarmnoneeabi/bin/../lib/gcc/arm-none-eabi/9.2.1/../../../../arm-none-eabi/bin/ld: .pio/build/Opulo_Lumen_REV3/src/src/gcode/gcode.cpp.o: in function `GcodeSuite::process_parsed_command(bool)':
/home/runner/work/Marlin/Marlin/Marlin/src/gcode/gcode.cpp:895: undefined reference to `GcodeSuite::M485()'
I'm not 100% sure why this is failing. It looks like the original failure started with a merged branch and now MarlinCore.cpp is conflicting. I'll look into it more and do some more debugging, but I'm hoping the issue is something obvious to you since I'm not in a state where I can compile and test this code and it visually looks correct to me.
Not sure how important this is, but I don't think this works with G-code compression in OpenPnP, which removes all spaces. When the data to send begins with a decimal digit (e.g.
M485 0...), the code number is no longer parsed as 485.
@dna-topoisomerase — To work around the problem you can enable GCODE_QUOTED_STRINGS and then wrap the value in quotes. For example:
M485 "0123456789ABCDEF"
@Jnesselr — This is now building and happy. I just needed to update features.ini to account for the updated source filtering. Is there anything else you wanted to do before this gets merged?
Yeah, we're working on some of the rs485 library features and one fix before merging. Thanks for the updates!
On Sat, Jul 1, 2023, 7:06 PM Scott Lahteine @.***> wrote:
@Jnesselr https://github.com/Jnesselr — This is now building and happy. I just needed to update features.ini to account for the updated source filtering. Is there anything else you wanted to do before this gets merged?
— Reply to this email directly, view it on GitHub https://github.com/MarlinFirmware/Marlin/pull/25680#issuecomment-1616249690, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALZPZZQP74T6LF2Q4K6FK3XODCSDANCNFSM6AAAAAAW7AR2FE . You are receiving this because you were mentioned.Message ID: @.***>
I recently received a donation of some CANBUS hardware to test with (FLY Mellow board, etc.). Some of the CANBUS stuff might be direct onboard serial. Some uses an SPI-to-CAN interface, which is a different animal. I'll know more once I get a change to play around with it.
Why didn't I find the relevant code and files related to RS485 when I downloaded Marlin bugfix 2.1. x
Why didn't I find the relevant code and files related to RS485 when I downloaded Marlin bugfix 2.1. x
Because this PR has not been merged yet.
Why didn't I find the relevant code and files related to RS485 when I downloaded Marlin bugfix 2.1. x
Because this PR has not been merged yet.
Hello, I would like to download the firmware for bugfix 2.1. x with RS485. What should I do?
Hello, I would like to download the firmware for bugfix 2.1. x with RS485. What should I do?
At the top of the page on all PRs is a link to the branch that contains the code:
Go there, then download the branch as a ZIP, as usual.
Adding a note here about this comment from @rondlh …
Here a project that connects a tool head via CAN bus. Both the motherboard and toolhead run Marlin based on ESP32. https://github.com/markniu/PandaCAN
I want to do something similar with a MKS Monster8 (STM32F407) and a MKS THR42 tool head (RP2040 based). This code: https://github.com/thinkyhead/Marlin/tree/bf2_wip_rp2040_skr_pico_PR is already very functional, I just have an issue running the onboard TMC2209 in the tool head, when I enable the driver it uses softwareserial, which causes a conflict on the RP2040 side, even the host communication breaks down.
Since it builds successfully we might as well go ahead and merge this to start the snowball rolling, eh @Jnesselr ?
There's a bug in the current version of the library that affects some special packets on some devices but it's only affecting Opulo stuff. What are your thoughts on my reverifying this from the Marlin side, merging, and then updating the library version at a late date?
I tried to connect PC with arduino ramps via M485 to avoid USB at all. PC- PCIe RS232_> rs232-rs485 converter ----2wires --- rs485 to ttl uart module connected to AUX1 serial. Firstly I tried module with DI DE-RE RO pins but did not find how to hack marlin to add DIR pin for communication direction. It transmits only in one direction. Next I tried module with only RX TX pins. and yes it works (even on 500000 bauds). Repetier is able to communicate with arduino. BUT. Suddenly communcation stops. unpredictably. And I need to reset board and reconnect repetier/pronterface. Or reconnect pronterface only. I tried notebook with usb-rs485 adapter. And got the same problem (112500) F.e. sending M43 several times. What I need to do to get it working?
I cannot force PC to halfduplex
My next task is to modify marlin to read temperature via rs485 from ECUC411 (modbus). pronterface log
SENDING:M503
echo:; Linear Units:
echo: G21 ; (mm)
echo:; Temperature Units:
echo: M149 C ; Units in Celsius
echo:; Filament settings (Disabled):
echo: M200 S0 D1.75
echo:; Steps per unit:
echo: M92 X100.00 Y100.00 Z500.00 E95.80
echo:; Max feedrates (units/s):
Got rubbish reply from COM19 at baudrate 250000:
Maybe a bad baudrate?
What are your thoughts on my reverifying this from the Marlin side, merging, and then updating the library version at a late date?
@Jnesselr — If an older version of the library still works we can hardcode the library version (or commit ID) and then when a future version of the library has the issue repaired we can update the library version. What versions of the RS242 library do we know work reliably?
It's a small issue with the packet protocol. We have a version we're pretty sure works, but it required some more changes on this side to get to work correctly. @sphawes should be testing that and will push up his changes to my branch and then we can be done with it.
To make it easier for control programs to parse RS-485 responses, it'd be nice for the various messages this can print to be self-consistent and consistent with the i2c commands. For example, right now this can return a variety of messages:
rs485-reply: TIMEOUT
rs485-reply: 001122334455
rs485-unexpected-packet: 001122334455
Error:String must contain an even number of bytes.
Error:String too long (32 bytes max).
Error:RS485: Write failed interrupted
Compared to i2c:
echo:i2c-reply: from:12 bytes:3 data:01 02 03
echo:Bad I2C address (8-127)
I propose the following:
echo:rs485-timeout
echo:rs485-reply: 001122334455
echo:rs485-unexpected-packet: 001122334455
Error:String must contain an even number of bytes.
Error:String too long (32 bytes max).
echo:rs485-error: interrupted
Note that messages starting with "Error:" may cause hosts like OctoPrint to halt and disconnect. If that is not desired behavior, messages should not start with "Error:". We now have the option to emit a less severe Warning: message.
We're definitely close, but still need to go over a couple of things.
@theacodes (Sorry, completely missed your message) I'd really like to avoid changing the output that is given with the rs485-reply, but mostly because of the extra work on the OpenPnP side. We already have OpenPnP regex reading off of that, and changing it here would require us to do a lot more work there to handle different types of replies. See here for the regex. We don't currently handle some of the other error conditions directly, we use OpenPnP retries to manage them.
If you can help figure out how to make the rs485 regex work in OpenPnP and @sphawes is okay with the support burden of that (meaning helping customers ensure what Marlin sends and what OpenPnP receives is the same), I'm game for it. Best guess would be rs485-(?<Value>.*) which might be too aggressive, but no worse than what is already in there.
The warning vs error thing is a little interesting to me. We don't go through OctoPrint for this, but I can understand why this functionality may eventually be something that's used through OctoPrint. Right now, other commands I looked at do send back errors but only if their input is bad. So some of our responses should still be errors, but the rs485_write_failed shouldn't be an error. If we change that one to like the echo:rs485-error: interrupted that @theacodes suggested, is that okay?
I am hesitant to tie implementation details here to OpenPnP, since it won't be the only host program using this functionality. @sphawes and I can write documentation for folks when they update their Lumen mobo FW.
My biggest concern with the output formats is just being consistent and being easy to match without having to do nested matching- the main problem being rs485-reply: TIMEOUT and rs485-reply: {data}. This means I can't solely match on rs485-reply.
I know I said I didn't want to tie this to the implementation details of any one consumer, but just as an illustration- the code from our internal internal tool, Glimmer, looks like this:
def _decode_response(line: str) -> bytes:
prefix, rest = line.split(":", 1)
rest = rest.strip()
match prefix:
case "rs485-reply":
if rest == "TIMEOUT":
raise RS485TimeoutError()
try:
return bytes.fromhex(rest)
except ValueError as err:
raise RS485ReadError(...) from err
case "rs485-unexpected-packet":
raise RS485UnexpectedPacketError(...)
case "Error":
raise RS485WriteError(rest)
case _:
raise RS485Error(...)
Where my preference would be this:
def _decode_response(line: str) -> bytes:
prefix, rest = line.split(":", 1)
rest = rest.strip()
match prefix:
case "rs485-reply":
try:
return bytes.fromhex(rest)
except ValueError as err:
raise RS485ReadError(...) from err
case "rs485-timeout":
raise RS485TimeoutError()
case "rs485-unexpected-packet":
raise RS485UnexpectedPacketError(...)
case "rs485-error":
raise RS485WriteError(...)
case _:
raise RS485Error(...)
I'm ok with needing to tie a Marlin version to a certain OpenPnP vacuum actuator read regex. It's a pretty simple copy-paste to update, and as long as we change both in lock-step, most folks will never encounter the difference. I definitely prefer to try to standardize our response format in an easy to parse way instead of preserving the current OpenPnP configuration.
If you do want to preserve a regex for the old pattern and also recognize the new pattern, I'm happy to help. I'm one of those mad nerds who enjoys making things like fancy regexes, proper CSS, and elaborate pseudo-justifications.
I believe we've sorted out the remaining questions about the protocol changes in this PR. If there are no other concerns I'll go ahead and merge this today.
Please hold off just a little longer. Several of us are at Open Sauce and Stephen needs to make a couple of small changes more.
thank you for waiting to merge @thinkyhead!
I talked through the implications of changing the format with @Jnesselr and @theacodes, and I think that it would be pretty frustrating for users if we change the response format. it's not just a simple regex change in OpenPnP as i thought in my previous comment; it would actually require a new build of OpenPnP to handle the new format correctly.
I propose we go back to the format in f1afaa1. @thinkyhead if this is acceptable to you, i can revert the format but keep your other changes and we can merge.
I propose we go back to the format
Will there be future updates to OpenPnP to allow for a different protocol format and additional features? If so, we should add a version specifier. The option #define M485_PROTOCOL 1 would build the old protocol, and version 2 would build the new (proposed) protocol that applies to some future version of OpenPnP. And as the OpenPnP protocol gets more changes and new features we can bump the version of the protocol to take advantage of those new features.
How about that idea?
That sounds great to us. Thank you @thinkyhead! I'll take my best crack at implementing this for your review if that works.