Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust forget commands to respect the direction of locos #233

Closed
wants to merge 2 commits into from

Conversation

nic547
Copy link

@nic547 nic547 commented May 4, 2022

For context: I'm trying to write my own software that interfaces with DCC++ EX.
There I need to deal with the fact that only a limited number of locos can be "active" at a time.
I would like to "forget" locos that aren't actually in use to keep the number of active locos down.
I don't think there's a good way to figure out which locos are parked on the layout and which ones have been removed from the layout, so my plan was to forget both, as a loco being "unknown" and not getting speed reminders is kinda the default anyway.

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?

  • Using DCC::setThrottle instead of DCC::setThrottle2 results in a call to DCC::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.

@Asbelos
Copy link
Contributor

Asbelos commented May 4, 2022 via email

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.
@nic547 nic547 force-pushed the directionAwareForgetting branch from ef009a0 to 39eefc7 Compare May 4, 2022 11:00
@nic547
Copy link
Author

nic547 commented May 4, 2022

Ok, I have adjusted that part. I extracted the calculation of the speed byte into it's own function, that I then use in the forget commands, so that I can call setThrottle2 and avoid the call to updateLocoReminder.

Is there an easy way to validate that the speed reminders aren't sent anymore?

@nic547
Copy link
Author

nic547 commented May 18, 2022

Superseded by #242

@nic547 nic547 closed this May 18, 2022
daniviga pushed a commit to daniviga/CommandStation-EX that referenced this pull request Mar 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants