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 find_pid/1 #108

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mnishiguchi
Copy link

This is an idea for minor enhancement.

I thought it would be nice to have a function to find pid by port name since we most likely know which port we are using. We can achieve it just by doing pattern-matching against the result of find_pids/0. What do you think?

@mnishiguchi
Copy link
Author

mnishiguchi commented Aug 29, 2021

It is nice to have but after all I did not use it in my project because I realized I could avoid starting twice for the same port at the higher layer using Registry.

@fhunleth
Copy link
Contributor

Since I intended find_pids/0 as a debug tool, I think that it can be changed or extended as needed to make life easier when debugging. I was just about to say that find_pid/1 would be fine to add, but then you said that you ended up not using it. Maybe we hold off with it for now.

@mnishiguchi
Copy link
Author

mnishiguchi commented Aug 29, 2021

As a debugging tool find_pid/1 should be OK, but yeah maybe we can wait until somebody else finds it necessary.

I personally decided not to use it in my code because I got confused when I used Circuits.UART (active mode) in a GenServer. I was able to avoid the :eagain error using existing find_pids/0 but then that GenServer stopped receiving the message from the UART. Maybe I did something wrong but after all, registering my processes in my own registry was easier for me in understanding what is going on.

P.S. Maybe the issue I had above was merely because the UART process does not know about my new GenServer process 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants