Need some advice on correct protocols

[OwenD]

I've been trying to delve into the firmware code a bit to try to help myself understand it in general, but also to learn a bit of C/C++ coding (only ever used Delphi).

As an experiment, I decided to try to implement some code to handle cancelling a PID autotune.
This is slated to be done using (probably) M303 H-1 (See here)
I picked this purely because it seemed like it should be achievable for a novice.
Plus as it's marked to be fixed anyway, I can compare notes down the track.

Looking at the source code it is evident that I need to make adjustment to Heat::TuneHeater() in heat.cpp

This is the code I added (which seemingly works fine)

if (seenHeater || seenTool)
	{
		// Added code to allow cancellatioin using M303 H-1
		if ((heaterNumber == -1) && (heaterBeingTuned != -1))
		{
			SwitchOffAll(true);
			reprap.GetPlatform().MessageF(GenericMessage, "Tuning cancelled by user : Heater %d\n", heaterBeingTuned);
			return GCodeResult::ok;
		}
		// End added code

		if (heaterBeingTuned != -1)
		{
			// Trying to start a new auto tune, but we are already tuning a heater
			reply.printf("Error: cannot start a new auto tune because heater %d is being tuned", heaterBeingTuned);
			return GCodeResult::error;
		}

So my questions are these

• I feel like there should be a check that the cancellation was successful.
  SwitchOffAll() doesn't return a result.
  It also doesn't reset heaterBeingTuned
  Searching for similar example I see calls to reprap.GetHeat().SwitchOffAll(true) when turning off heaters. reprap.GetHeat seems like it should return a pointer, but I can't figure out how I would use it and none of the other calls to that function appear to check for success.
  Am I just overthinking this section?
  Is it assumed that SwitchAllOff() will always work?

• Assuming I create some useful code, don't burn the house down and want to keep it in the firmware (even if it's not useful to others), what is the correct way of merging future changes of main repository with my "custom build"
  I'm really not up to speed on how GitHub works when you're not part of an official team.
  I can see that Eclipse has functionality inbuilt, but I assume it requires you be part of a development team.
  I presume I would need to create a branch, but again I'm unsure of the protocols.
  Can anyone create a branch and is there any risk in doing so?
  The documentation on this is pretty heavy going.
  Plus I don't expect to be submitting pull requests any time soon, so I want to make sure my curiosity isn't going to cause issues to the proper developers.

[dc42]

@OwenD function reprap.GetHeat() returns a reference, which is like a pointer except that dereferencing is automatic without having to use the * operator. See for example https://www3.ntu.edu.sg/home/ehchua/programming/cpp/cp4_PointerReference.html#:~:text=A reference variable provides a,value stored in a pointer.

I agree that SwitchOffAll should reset heaterBeingTuned. I will fix that in the 3.5-dev source code.

I think the reason that SwitchOffAll doesn't return a result is that switching off a heater on controlled by the main board never fails (barring hardware failure). Arguably it should return success/failure now that we can have heaters controlled by CAN-connected boards, but I'm not sure what we could do if it failed other than print a message, which the code in RemoteHeater::SwitchOff already does.

[OwenD]

@dc42
Thanks for taking the time for this detailed response.
I appreciate that you've got better things to do than teach noobs.