-
Notifications
You must be signed in to change notification settings - Fork 29
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
Fixes a standards compliance issue with the alias conflict handler. #793
Conversation
The standard requires that at the time when an alias conflict is detected, the device to issue an AMR frame with the conflicted alias. This PR adds the missing AMR frame. Adjusts tests to heck for this frame. Fixes a bug in the Global AME handler that was unexpectedly emitting AMD frames for removed aliases, because the cache entry was not correctly wiped when the remove was processed, and the iteration also picked up such deleted entries.
src/openlcb/IfCan.cxx
Outdated
@@ -170,6 +175,37 @@ class AliasConflictHandler : public CanFrameStateFlow | |||
return exit(); | |||
} | |||
|
|||
/// Sends an AMR (alias mapping release) frame with the alias alias_ and | |||
/// the loca node ID that we have for it. Then deletes the mapping from the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
loca -> local
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The standard requires that at the time when an alias conflict is detected, the device to issue an AMR frame with the conflicted alias.
This PR adds the missing AMR frame.
The standard says to send the AMR if the alias is for a node in Permitted state. As I read it, if that alias hasn't been used yet to go to Permitted state, it's not really "in use" and you just drop it.
Does this PR do that?
…ate an AMR frame upon conflict.
an AMR frame upon conflict.
There was indeed a bug there. Thanks for flagging! |
The standard requires that at the time when an alias conflict is detected, the device to issue an AMR frame with the conflicted alias.
This PR adds the missing AMR frame. Adjusts tests to heck for this frame. Fixes a bug in the Global AME handler that was unexpectedly emitting AMD frames for removed aliases, because the cache entry was not correctly wiped when the remove was processed, and the iteration also picked up such deleted entries.