Spin out driver subclass

[zsyed91]

• Create a Driver subclass
• Added more tests to consider some edge cases and sad paths
• Test coverage is 100%
• Use FileWatcher instead of IO.select to prevent 100% cpu usage when waiting for value to change. Method now blocks until value is updated.

[elmatou]

Unless you changed PiPiper's code #driver= need a subclass of Driver, not an instance.

[zsyed91]

Yes you are correct, forgot to remove the .new. I will fix this.

[elmatou]

Hi @zsyed91,
My thoughts lies in the comments above.
I'm only concerned on your #wait_for_change implementation, maybe you can explained me what pushed you to this direction.

[zsyed91]

@elmatou see my note in the code regarding FileWatcher. We can definitely discuss it further, it is a big change, after all.

[elmatou]

Hi @zsyed91,
tell me when your are good to go with your PR.
I just succeed to use I2C with sysfs & bcm2835. This will be a nice addition. see you

[zsyed91]

@elmatou Yeah I have been insanely busy this past week with some life stuff. I will push the suggestions soon. Glad you tested I2C with the code and that it works 😄

[elmatou]

My bad, select was and should listen to value_file of course, edge_file is only for setup.

As far I understand, FileWatcher use modification time, I get through the code and only found this line referring to the file :
https://github.com/thomasfl/filewatcher/blob/master/lib/filewatcher.rb#L125

as for select(2) not blocking on read. We need to investigate more, the SO thread you mentioned was listening on the reading part of the file descriptor and not the error part hence the difference in behaviour.
looking forward to read you.

[zsyed91]

Sure I can take a look again. I will have to look at the difference between checking errors vs just reading via select. If it helps, I can pull that logic out of this PR and open a separate one containing those changes.

[elmatou]

I think it would be best to keep atomic/feature oriented PR. I will
continue to investigate on the matter.
On Apr 22, 2016 4:54 AM, "Zshawn Syed" [email protected] wrote:

Sure I can take a look again. I will have to look at the difference
between checking errors vs just reading via select. If it helps, I can pull
that logic out of this PR and open a separate one containing those changes.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#4 (comment)

[elmatou]

Hi @zsyed91,
where are we on this since we last spoke ?
do you need a hand to spin out the drivers ?

[zsyed91]

@elmatou Hey, sorry I just switched jobs and am currently moving cross-country so this slipped through the cracks. I just pushed the changes. I fixed the driver class and removed the pin_wait_for changes so that I can open it in a separate PR.

[elmatou]

Ok, thx you, I'm gonna do a full review of the PR, and it will be good to merge

[zsyed91]

Awesome, let me know and then I will merge if everything is good to go 😄

[elmatou]

you are right, we should remove this.

[zsyed91]

right, once we release 3.0 officially I can open a new pr to remove this.

[elmatou]

I just pointed to 2 methods not useful for now.
and maybe we could set the dependency to pi_piper 3.0 (if 3.0 is good for you.).