Skip to content

Separation of RTD and TC classes confusing #14

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

Closed
JHSawatzki opened this issue May 16, 2024 · 1 comment
Closed

Separation of RTD and TC classes confusing #14

JHSawatzki opened this issue May 16, 2024 · 1 comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement

Comments

@JHSawatzki
Copy link

JHSawatzki commented May 16, 2024

Both probe types use the same inputs/channels so the current implementation is confusing, espacially when using both types in the same project.

I suggest using a shared class like I did in my fork of the old lib, see https://github.com/JHSawatzki/Arduino_MachineControl/blob/master/src/Arduino_MachineControl.h

I could probably also provide a pull request with an implementation not breaking your current API.

@leonardocavagnis leonardocavagnis added type: enhancement Proposed improvement topic: code Related to content of the project itself labels May 17, 2024
@leonardocavagnis
Copy link
Contributor

Hi @JHSawatzki,
Thanks for the proposal!
I don't completely agree with your point. We moved to separate classes to avoid using the "double dot" notation (temp_probes.rtd.) of the deprecated one.
However, I invite you to propose your solution in a PR, and I'll analyze it. 😊

@leonardocavagnis leonardocavagnis linked a pull request Jun 3, 2024 that will close this issue
@leonardocavagnis leonardocavagnis closed this as not planned Won't fix, can't repro, duplicate, stale Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants