search

chrizog / someipy

A Python Library implementing the SOME/IP Protocol
https://chrizog.github.io/someipy/
GNU General Public License v3.0
26 stars 5 forks source link

Implementation error in someip_sd_builder.py #14

Open dayangshanbaoye opened 2 weeks ago

dayangshanbaoye commented 2 weeks ago

While reading someip_sd_builder.py, I found a few parts that are a bit hard to understand, and I think they might be errors. Could you please take a moment to review them?

Error 1: The internal code of the build_offer_service_sd_header method and the build_stop_offer_service_sd_header method is identical. Would it be possible to merge these methods and use ttl to differentiate between offering and stopping the offer? Additionally, in the build_stop_offer_service_sd_header method, SdEntryType.OFFER_SERVICE is used instead of SdEntryType.STOP_OFFER_SERVICE.

Error 2: Should the build_subscribe_eventgroup_entry method be renamed to build_subscribe_eventgroup_sd_header? Since it also returns a SomeIpSdHeader object, this name might be more appropriate.

Error3: Should the build_find_service_sd_header method be implemented?

chrizog commented 1 week ago

Hi, thanks for your investigations.

  1. Yes, you're right. However, I would keep two differently named functions for clarity instead of using TTL only, in order to make the intention clear. But the duplicate code can be extracted into an internal function.
  2. Yes, you are right. It can be renamed.
  3. Yes, it could be implemented. However, until now no "Find Service" entries are sent in the someipy library. I would suggest to implement and test the build_find_service_sd_header method as a part of the task to integrate the "Find Service" entries and logic.

Do you want to contribute? If you want, you can fix the mentioned points and assign the issue to you.

dayangshanbaoye commented 1 week ago

Do you want to contribute? If you want, you can fix the mentioned points and assign the issue to you.

I'm glad to contribute, thanks for your invitation