[WIP] Change gem structure

[zsyed91]

🚧

Note to self: test this on the Pi first

[elmatou]

It seem's you pushed 3 hours before my comment on sysfs.
I'm not far from a complete implementation with specs on bcm2835.
How do you want t proceed ? do I finish it now, or do you want to do your thing before it ?

[zsyed91]

No please continue. I wanted to see if we could get the driver in a test-able state (by stubbing the ffi call). I was able to do so but feel free to continue. I will work this branch around your code. A lot of what I implemented is sysfs code anyway. How about after merging #4, I will merge this one. Then we both can continue working in parallel. If not I can wait until you finish everything and then merge this branch.

Does that sound reasonable to you?

[elmatou]

I think it is a missunderstanding.
PiPiper.driver in my implementation is the only instance of any driver kind loaded at any moment.
I even though a moment to use the singleto module : http://ruby-doc.org/stdlib-1.9.3/libdoc/singleton/rdoc/Singleton.html

If we want to be able to have several drivers for several Object, we have to rethink the implementation. I don't think it is very useful, but we can discuss it.

[zsyed91]

Yeah I thought we would do something like this in the user code:

require 'pi_piper'

require 'pi_piper-sysfs'
# or
require 'pi_piper-bcm2835'

PiPiper.driver = PiPiper::Sysfs.driver
# or
PiPiper.driver = PiPiper::Bcm2835.driver
# ... do stuff ...

And yes, only 1 driver would be loaded for PiPiper at any given moment.
Is that what you meant, or something different?

And then later on if you do

PiPiper.driver = PiPiper::Sysfs.driver # => raise 'driver already loaded'

Or we could allow driver changes but that would complicate things.

[elmatou]

in PiPiper I wrote :

  def driver=(klass) 
    if !klass.nil? && (klass <= PiPiper::Driver)
      @driver.close if @driver
      @driver = klass.new
    else
      raise ArgumentError, 'Supply a PiPiper::Driver subclass for driver'
    end
  end

It ensure the driver is properly closed before loading a new one. I thought it was enough, it free a bit of memory, and it lightened the at_exit hook.

[elmatou]

And as you only need one driver to be loaded my implementation allow to only write :

require 'pi_piper'
require 'pi_piper-bcm2835'

it load the code and load the driver with PiPiper.driver = PiPiper::Bcm2835 in the Bcm2835 module/class

[zsyed91]

Ah I see, I can certainly change my code so that it works like the above. It will load the driver when running require.

[zsyed91]

@elmatou what are your thoughts on the new modules for Pin, SPI, and I2C? Please feel free to make any comments on my code as well 😄

[elmatou]

I'll prefer the included(base) hook for these kind of setup

BTW even without your workaround with attach_function I can have very good coverage (for now 270 / 289 LOC (93.43%))

[elmatou]

Your module approach is a good idea, it parts code in meaningful segment.
It seems to be a bit too much for our little sysfs driver but it is the way to go when implementing bcm2835 or WiringPi (maybe next month). I just don't understand why your are creating a setup_xxx method ? you mentioned coverage but I do have a good coverage in my implementation (>93%) without it.

[zsyed91]

Yeah coverage was good before, I just wanted everything covered. I think I may rewrite the tests for the setup method. The other thing I wanted to do was to put the attach_function and ffi_lib calls into their own method. That way we can run rspec without having travis fail because it can't open the .so file. And the attach_function methods need the ffi_lib defined first. So the way it is now, you can run the tests without worrying about loading the actual driver. Those can be done through integration tests on the pi itself.

I am also a fan of leveraging OOP rather than procedural programming which is sort of how it was before.

[elmatou]

libbcm2835 are often used in different driver methods, anyhow I don't think it is a good practice to to do (dynamic assignement) with methods, it is prone to error.

What you can do, and is often done when testing ffi based gems, is to stub the ffi_lib and attach_function by overwritten them before requiring Bcm2835 typiclay in a spec_helper.

To be more pragmatic, do we really need to have travis-ci for this particular part of pi_piper ? (yes, there is pre travis-ci area and it was not dark ages !)

[elmatou]

not useful in production, could be added for dev

[zsyed91]

absolutely, it was just a placeholder since this PR was a work in progress.

[elmatou]

Hi,
I think, I get your point with the modules split strategy.
Will you let me redo it for this lib ?
I'm not sure your dynamic declarations will work easily. I can try to stub the #ffi_lib declaration, and #attach_function as well.

[zsyed91]

@elmatou Hey, sure feel free. This is/was a work in progress anyway but if you want to take a crack at it go ahead. I will focus on getting out 3.0.