Duet3D / DuetSoftwareFramework

Software framework for the next-generation Duet3D platform based on .NET
GNU General Public License v3.0
61 stars 28 forks source link

M550 is ignored #34

Closed wilriker closed 5 years ago

wilriker commented 5 years ago

duetcontrolserver=1.0.2.1-1 duetruntime=1.0.2.1-1 duetsd=1.0.1-2 duetsoftwareframework-meta=1.0.2.1-1 duettools=1.0.2.1-1 duetwebcontrol=2.0.0.RC8.b1-1 duetwebserver=1.0.1.1-1 duet3firmware=3.0b6-ch@2019-07-21b1

Instead of using the name of the device set via M550 DWC will display the hostname of the RPi that is serving DWC.

chrishamm commented 5 years ago

There should be no difference between the SBC hostname and the machine's (RRF's) name. But the hostname cannot be updated via M550 yet so it may be worth considering to make M550 change it.

wilriker commented 5 years ago

That won't work since the documentation of M550 states

The name can be any string of printable characters except ';'

and there are several invalid characters inside the range of printable ones when it comes to valid hostnames. So in that case M550 would also have to be limited to a subset of printable characters.

chrishamm commented 5 years ago

OK, I'll add support for it in the next DCS version.

wilriker commented 5 years ago

Fixed in DCS 1.0.3.0.

wilriker commented 5 years ago

And reopening it again. DWC does show the correct value unless M550 breaks (see below). But it also tries to update the RPi's hostname. This can lead to serious connection issues because it will just do weird things to the hostname:

Example

hostname = farm-12-printer-1-23 M550 = Farm 12/Printer 1-23

First of all this will kill DCS and set the hostname to MyDuet (DWC interestingly shows "My Duet" with a space). And even if it worked it would have garbled the hostname of this machine that also might be relevant for internal DNS routing.

I strongly recommend to leave the hostname alone.

EDIT: When using the above M550 value that leads to DCS being killed the log has the following message

Aug 05 16:36:42 My DuetControlServer[4849]: SPI task terminated
Aug 05 16:36:42 My DuetControlServer[4849]: [crit] DuetAPI.Commands.CodeParserException: Duplicate 1 parameter
Aug 05 16:36:42 My DuetControlServer[4849]:    at DuetAPI.Commands.Code.Parse(TextReader reader, Code result, Boolean& enforcingAbsolutePosition) in /home/christian/duet/DuetSoftwareFramework/src/DuetAPI/Commands/Code/Parser.cs:line 328
Aug 05 16:36:42 My DuetControlServer[4849]:    at DuetControlServer.FileExecution.BaseFile.ReadCode() in /home/christian/duet/DuetSoftwareFramework/src/DuetControlServer/FileExecution/BaseFile.cs:line 106
Aug 05 16:36:42 My DuetControlServer[4849]:    at DuetControlServer.FileExecution.MacroFile.ReadCode() in /home/christian/duet/DuetSoftwareFramework/src/DuetControlServer/FileExecution/MacroFile.cs:line 140
Aug 05 16:36:42 My DuetControlServer[4849]:    at DuetControlServer.SPI.ChannelInformation.ProcessRequests() in /home/christian/duet/DuetSoftwareFramework/src/DuetControlServer/SPI/Channel.cs:line 191
Aug 05 16:36:42 My DuetControlServer[4849]:    at DuetControlServer.SPI.Interface.Run() in /home/christian/duet/DuetSoftwareFramework/src/DuetControlServer/SPI/Interface.cs:line 435
Aug 05 16:36:42 My DuetControlServer[4849]: [warn] Connection error: System.Threading.Tasks.TaskCanceledException: A task was canceled.
Aug 05 16:36:42 My DuetControlServer[4849]:    at DuetControlServer.IPC.Processors.Subscription.Process() in /home/christian/duet/DuetSoftwareFramework/src/DuetControlServer/IPC/Processors/Subscription.cs:line 96
Aug 05 16:36:42 My DuetControlServer[4849]:    at DuetControlServer.IPC.Server.ConnectionEstablished(Socket socket, Int32 id) in /home/christian/duet/DuetSoftwareFramework/src/DuetControlServer/IPC/Server.cs:line 86
chrishamm commented 5 years ago

So for exactly this reason I did not want to add support for M550. The only clean solution for this would be to remove the "hostname" system call and to permit only hostname parameters that have an equal subset of letters/digits of the SBC's configured hostname. In addition, support for M550 from non-SBC input sources should be removed since RRF does not have the possibility to verify the RaspPi's hostname.

I understand the point to be able to configure a "pretty" hostname via M550 but I don't think it would be good to make DCS configure any more than that (as you can see, obviously).

wilriker commented 5 years ago

@chrishamm I don't think I conveyed my intention here clearly. Sorry for that.

What I meant: leave the SBC hostname alone. The user can/should manage that. But use the name set by M550 to be the name displayed in DWC. Nothing less, nothing more.

I don't see a technical necessity to keep both names in sync. Maybe you can elaborate on that a bit if there is something I am missing here.

T3P3 commented 5 years ago

There is the use case of connecting to your printer as "hostname.local" where "hostname" in the past used to be the name of the printer. Because of that I think it is best to restrict the set of valid printer names to valid hostnames. This is because from a user perspective, their primary interaction with the printer will be over DWC (not via SSH into the SBC or whatever) so when they change their "printer name" they would expect the name to change in DWC, and to go to "newprintername.local" for example.

On Duet 3 I see the process of changing the printername something like:

1) User selects new name. User informed that it will change the hostname of the printer. 2) Name is checked for validity against hostname rules. 3) DSF attempts to change SBC hostname, If successful, change reported printername in RRF (M550), redirect browser to new hostname if connected using mDNS. If unsuccessful, change nothing and report back to the user.

If there is a requirement for more information in the printer name than a valid hostname will accept then a "description" or "short description" can be added to M550.

While it is not intuitive yet, we need to see the SBC and the software running on it as part of the "printer" from the user's perspective.

wilriker commented 5 years ago

@T3P3 At least when using Arch Linux I don't find the SBC connected to Duet 3 neither as <hostname>.local nor does

$ avahi-browse --all --ignore-local --resolve --terminate

list anything related to Duet 3 SBC (it lists the Duet 2 Wifi in the same network though). I don't know how that is in Raspbian. If that's the same then the user would first have to configure advertisement/discoverability of the service via Bonjour/zeroconf/what-so-ever.

T3P3 commented 5 years ago

Yes its not implemented by default in Duet 3. But that is fairly usual on Duet 2 using Wifi.

Eventually we will have SBC images with everything setup (or maybe printer manufacturer's will have these), so that its plug and play. the SBC image at that point should set itself up to be discoverable with its default hostname on the local network. Most users will change that when they connect to it...

wilriker commented 5 years ago

IMHO this is a try to fit the SBC into an embedded-structure as it was before. Also the users will most likely in the end still have to communicate via SSH with the SBC or they won't be able to update the OS/packages. I don't like to have taken control away from me. But others might see that different.

Getting back to topic: I would find an extension of M550 acceptable that the current Pnnn parameter sets a readable name (displayed in DWC) and have a new parameter e.g. Hnnn that sets the hostname. In case of absence of the latter the hostname could be derived from the value of Pnnn. This way the user still has the choice to keep them separate.

T3P3 commented 5 years ago

This won't prevent you SSHing in and doing anything you like.... this is just a default that will feel normal for people who don't want to deal separately with the SBC as part of the printer. Unless something goes wrong the average user will connect to the printer and manage everything through DWC, or some other UI. They will do this either via a touchscreen or over the network. This is not to say thats the only way we should support - just expect it to be the default.

The proposal works, although i would slightly prefer it that M550 Pnnn sets the hostname and the printer name (subject to the checks already specified). and that any other switches were used to set the short description and possibly to override the default behaviour (for example to break the host name to printer name link.

wilriker commented 5 years ago

I know that this won't prevent me from doing anything. But if I set the hostname in the OS to my liking and then also have a M550 Pnnn in my config.g with a better readable name that whenever executed overwrites the hostname this can be annoying. That's all I try to convey here.

Your idea is fine for me. I am happy with any solution that lets me set hostname on the SBC and display name in DWC separately or gives me the possibility to not have the hostname altered.

T3P3 commented 5 years ago

Yes I understand, I the discussion is just around what is the preferable default.

A) Unconnected by default: M550 Pmachinename sets the machine name as it does now. M550 Hhostname sets the hostname. M550 Pname S1sets the machine name and the hostname to name.

B) Connected by default: M550 Pname sets the machine name and the hostname to name. M550 Phostname S1 sets the hostname, without setting the machine name. M550 Pmachinename S2 sets the machine name without setting the hostname.

I don't mind either to the extent that the configurator can provide the default that they are the same. and DWC can have an option (normally selected) to change them both at the same time.

chrishamm commented 5 years ago

I changed DCS 1.0.3.1 and the latest RRF versions according to my proposal. Now M550 allows you to choose a pretty label to display on DWC as long as the numbers/digits in it are equal to the configured hostname as set by hostname (/etc/hostname).

If we decide to go with an own Linux distribution, we can still write a DSF plugin to reconfigure the system hostname on demand.

dc42 commented 5 years ago

M550 was introduced so that the host name of the web server on the Duet can be set. When the web server isn't running on the Duet, the host name should be set by whatever mechanism the Pi offers. So my preference would be for M550 to be not supported, but if someone runs it then it should provide a helpful error message. Also we should extend the documentation of M550 on the GCodes wiki page to describe how to set the host name on a Pi.