Adjust forget commands to respect the direction of locos #233
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the "forget locos" commands send e-stops without the direction bit set, e.g. they're backwards e-stops.
When a loco is still on the layout, this can lead to the headlights facing the wrong way, which is a bit unfortunate IMHO.
With this PR, instead of sending the "backwards e-stop" packet to locos the CS will send the e-stop packet with the direction the locos should already have.
While I personally only require the change to the "forget this loco" command, I have also adjusted the "forget all locos" command to be consistent. Instead of sending a broadcast e-stop, it now only sends the e-stop to locos in the speed table.
DCC::forgetLoco
calls the setThrottle function twice - I'm not sure why, but assumed it's to ensure that the loco actually gets the packet, since it's not gonna get any reminders.DCC::forgetAllLocos
doesn't send it twice. Should I adjust one of them?UsingDCC::setThrottle
instead ofDCC::setThrottle2
results in a call toDCC::updateLocoReminder
. If I understand the code correctly, calling the updateLocoReminder with for a loco that isn't in the speedTable should do nothing. Is it ok to rely on that behaviour?This PR is probably a breaking chance, while not explicitly documented I would assume that other software might rely on the current behaviour (at least mine does). So it's not quite obvious if this change is worth it or if there are better options.
Also thanks to haba on discord for helping me a bit with this.