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

add the possibility to return all the possible solutions using the from_sample_point static methods #1671

Merged

Conversation

danielhrisca
Copy link
Contributor

The CAN databases can contain restrictions for the timings:

  • minimum and maximum sampling times
  • minimum and maximum SJW values
  • minimum and maximum number of time quanta

With this change the user can get all the possible solutions for a given sampling value and can further filter the solutions using the known constraints.

@zariiii9003
Copy link
Collaborator

Both me and mypy do not like the different return types of the from_sample_point method. 😄

What do you think about adding another method which generates BitTiming instances:

    @classmethod
    def iterate_from_sample_point(
        cls,
        f_clock: int,
        bitrate: int,
        sample_point: float = 69.0,
    ) -> Iterator["BitTiming"]:
        ...
        yield bt
        ...

The from_sample_point method could use the generator to create the possible_solutions list and return the "best"

@danielhrisca
Copy link
Contributor Author

@zariiii9003 thanks for the pointers, let me know if the new code is ok

@zariiii9003
Copy link
Collaborator

Looks good, could you add some trivial test?

Copy link
Collaborator

@zariiii9003 zariiii9003 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you 👍

@zariiii9003 zariiii9003 merged commit b2689ae into hardbyte:develop Oct 12, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants