Fixed issue where the SimpleService stop_announce method would crash #23
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi! Thanks a lot for your contribution. The original code is even more wrong than what you describe - in |
|
Ahh OK yeah I missed that. I guess my explanation was also wrong as I though the python remove method worked by reference rather than value. Glad to be able to help. |
|
Hey, just wondering if there's anything I can do to help get this merged? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I've noticed that calling the
stop_announcemethod on the SimpleService class causes the program will crash.The crash is caused by the
stop_announcemethod re-constructing the instance so that when the announcer searches through it's list of services it is announcing it is unable to remove the service instance on this line:https://github.com/afflux/pysomeip/blob/master/src/someip/sd.py#L1313
If you simple call the
stopmethod of SimpleService this issue is not encountered by SD announcer won't stop announcing the service.In in the toos/simpleservice.py file this is circumvented as the entire announcer is stopped before the SimpleService is stopped.
I've made an example of this here: https://github.com/AlwinHughes/pysomeip/blob/demo/tools/simpleservice.py. This demo will crash if it's run with the version of SimpleService that is on the current afflux/master but completes successfully with the changes in this PR.
My proposed solution is to store the service_instance when SimpleService
start_announceis called. Then whenstop_announceis called the correct instance can be passed to the announcer. This allows multiple SimpleServices to be announced by the same ServiceAnnouncer and each service to be offered/stopped independent of each other.I'm new to contributing to open source projects so please let me know what you think or if I've missed anything.