SwitchEV / iso15118

Implementation of the ISO 15118 Communication Protocol (-2, -20, -8)
Apache License 2.0
147 stars 82 forks source link

Unclear documentation in is_contactor_opened #406

Closed raoulKalkman closed 1 month ago

raoulKalkman commented 1 month ago

This is about the functions in iso15118/secc/controller/interface.py

The functions are written as if they are checks for the contactor, however the documentation is unclear.

Especially the is_contactor_opened confuses me while making an implementation.

I would suggest either changing the function name in case it actually opens the contactor (this would surprise me) or writing a doc as something like "Check if the contactor is opened. (open contactor -> terminated energy flow)"

That of course is a draft, but I hope you get the gist of it.

    @abstractmethod
    async def is_contactor_opened(self) -> bool:
        """
        Sends a command to the SECC to get the contactor status is opened to terminate
        energy flow

        Relevant for:
        - all protocols
        """
        raise NotImplementedError
    @abstractmethod
    async def is_contactor_closed(self) -> Optional[bool]:
        """
        Sends a command to the SECC to get the contactor status is closed

        Relevant for:
        - all protocols
        """
        raise NotImplementedError
shalinnijel2 commented 1 month ago

Hi @raoulKalkman, Yes, the comment is misleading. Thank you for pointing this out. Have updated it in master now. PR. Kind regards, Shalin.