search

NEEOInc / neeo-sdk

NEEO Brain SDK
https://neeoinc.github.io/neeo-sdk/
MIT License
48 stars 17 forks source link

NEEO - Driver Manager #74

Closed Shepless closed 6 years ago

Shepless commented 6 years ago

As requested by @bdauria in this Planet NEEO forum post

I've got the Driver Manager in a beta state - repo is here. I have added a brief todo list in the README so you can see the direction I'm heading with it. Hopefully the engineering team will find this of some use. Let me know if you have any questions.

I've uploaded a demo video here

bdauria commented 6 years ago

Thanks a lot for your efforts @Shepless, this looks very promising. Having a GUI is definitely something that users without a software development background will find useful. I think it would make sense to maybe keep everything related to the backend in this repository, and separate the UI to another one. Can you think of a list of things related to the SDK that you would see integrated to our repository?

Our next release will for instance include the possibility to expose drivers for re-use in other SDK projects, along with the automatic server start with the found drivers.

TombolaShepless commented 6 years ago

@bdauria No problem - it's been keeping me out of trouble for a couple of days. I've kept the server/client implementation in the same repository (as of now) because I want to provide a simple entry point for non-developers (i.e. exe, dmg, electron app). If I split them out this becomes a lot tricker for not much benefit. This is just where my thinking is right now but nothing is set in stone.

As far as SDK usages goes, right now in that repo it doesn't need it at all. In short all the manager is doing:

So at the moment the only thing related that might be worth integrating is the npm search and install? Possibly the process management, but I'm not sure if pm2 is "overkill" for a leaner production solution.

bdauria commented 6 years ago

@Shepless Yes I agree, it should have only one entry point. I was more thinking of separating the SDK-related code from the UI, so the official SDK, and other developers directly relying on it can benefit from it.

The process management is for instance a good example of what we could integrate to the official SDK, as for the next version, the recommended way will be one server instance for multiple devices, which can lead to a lack of overview, especially if one device is responsible for a crash of the server.

In short: my idea is to leverage the SDK as much as possible, and try to enhance it with what's missing at the same time.

But also, this thread is more a discussion than an implementation plan, so feel free to come up with your ideas :)

TombolaShepless commented 6 years ago

@bdauria Sounds good, I'm not massively familiar with the SDK itself yet. When my NEEO arrives I will certainly be playing with it to get a better idea of how everything ties together. Until then I will carry on with my current plans.

When 0.50 is out with the CLI and single instance/multi drivers is out I'll fork this repo and start playing around with the process management etc.

Shepless commented 6 years ago

@bdauria do you guys have a branch pushed up for v0.50? Keen to see what the new CLI work is looking like so I can start thinking about how we can integrate.

Shepless commented 6 years ago

@bdauria so my NEEO arrived yesterday and I have been playing with the SDK a lot. I have some ideas around settings that I think might be useful. I can push up a driver showing a rough idea of what I'm working on if you like? I was also curious as to whether you guys accept PRs on here?

neophob commented 6 years ago

@Shepless sure - we accept PR's and like new ideas! Thanks

Shepless commented 6 years ago

@neophob awesome! So rather than try to explain the settings idea I have I will push up a driver I have been working on and will document in there. Just to get a feel from the dev team if this a direction they would want to go in, rather than invest the effort in something thats just a non-starter. I'll ping you when it's up (should be shortly!).

Shepless commented 6 years ago

@neophob @bdauria here you go let me know if it doesn't make sense. It's hard to visualise without the functionality living in the right places (i.e. inside the SDK)

neophob commented 6 years ago

fyi: updated SDK v0.50.3, check out the next branch

TombolaShepless commented 6 years ago

@neophob nice! Just a question on the update. Won't this line cause the CLI to try and load the neeo-sdk as a driver? Did you get a chance to look at the settings proposal?

neophob commented 6 years ago

@TombolaShepless nop because we also check for a devices/index.js file. but I agree, the check today is rather crude.

unfortunately not yet - but I added it on my list. I'll check with @bdauria.

Shepless commented 6 years ago

@neophob do you have any feedback on the settings implementation? If you do I can start putting a PR together for the next branch

neophob commented 6 years ago

@Shepless we'll come back to you, expect some feedback

neophob commented 6 years ago

@Shepless no in-depth feedback yet but i like the idea that we use your driver manager as easy interface for the users to include different community drivers. this would make it definitive easier to install drivers. also including it as sdk driver itself might be an idea we should focus on

TombolaShepless commented 6 years ago

@neophob thanks for taking a look. Yeah the "inception" style DriverManager driver is definitely something I've been thinking a lot about. It could possibly allow for driver installation via the remote/app/web interfaces. Which would be great. Did you have any thoughts on the settings proposal too?

neophob commented 6 years ago

Here's my high level view and goals I see. this are not necessary technical steps - but more a brainstorming

Goal 1

I think the first step should be, that non developers can easy install and use community drivers using the webapp you created.

Open steps:

Goal 2

Make it easy to add/remove community drivers. however adding a device might be a two step process (select and add community driver + search for a new device). so this is not straightforward for the non experienced user. How can this be improved?

bdauria commented 6 years ago

@TombolaShepless Can you create a PR on the next branch with the settings feature? It's going to make it easier for us to give you our feedback. But already, what comes on top of my head:

It's a pretty good idea do have a declarative way of defining the drivers, that'll reduce a lot of boilerplate. I'm not sure why the settings definition should be a two way process though. Can't we leave the settings either in package.json, or in the dedicated settings.json as part of the committed source? We can still provide useful error messages if the format is not correct or something is missing, and we don't force the user to use yet another command.

the next branch already uses the package.json to store some settings, such as the brain host, port or server name.

pfiaux commented 6 years ago

I wonder if this should be part of the neeo-sdk or part of a separate package. The way I see it they solve different use cases: the neeo-sdk is the run time component you need to connect the driver to the Brain, the driver manager (and the cli) are tools on top of that.

In a way if the driver manager installs packages from npm and these packages use the neeo-sdk it will be installed by following the dependencies. If it's all in neeo-sdk it gets quite large.

Shepless commented 6 years ago

@bdauria I think I agree with @pfiaux here. In my head the SDK should expose the tools required to build drivers and start servers. But I don't think it should actually start any servers or try and automatically import drivers - I think that should be left to the consumers of the SDK. To try and use an analogy here:

Express allows you to build APIs/Websites, it has tools to let you do this. However it doesn't automatically register middleware, or setup routing, or authentication, or even start a server - its a blank slate to start creating whatever server you want. I think we should treat the SDK in the same way - give everyone the tools they need to build whatever driver(s) they want and nothing more. Whilst this isn't user friendly for non-technical users, it is where the community step in. In my head the below would be the packages

From those low-level packages, the community can start to build the more user friendly eco-system around it (e.g. drivers, driver managers etc.). I think if you guys try to focus on the end user too much with this package you will run into a scenario where you need to support all the weird and wonderful use cases/knowledge levels and that just isn't feasible and it will turn very messy, very quickly. I think focusing on the lower level stuff and taking out the CLI, driver import and start server functionality gives us the following:

In all honesty, the reason I bought the NEEO was because it allows me to build whatever I want for it. We need to be careful that we don't restrict how the NEEO developer community can help build the ecosystem around the NEEO product family. This could be the key to rapid NEEO adoption as the more developers that are active and engaged, the more features you guys get with way less time and effort. You're essentially providing a platform for an eco-system to naturally grow and expand - which is a win-win scenario.

So as a high level diagram:

neeo_sdk_architecture 1

As @nklerk mentioned below - if the community built drivers/tools are valuable and are widely adopted then there is no reason why they can't eventually by integrated into the "official neeo" area of the above diagram for neeo to official maintain, support and progress. They would just be separate "products".

To address the settings question directly @bdauria:

I'm not sure why the settings definition should be a two way process though

It would be a one step process for the developer and also a one step process for the user of the driver. A real world example:

Using the above scenario, I can easily define in neeo-driver-plex-client package.json that an end user must have a settings.json file with the following structure, in order for the neeo-driver-plex-client to function correctly for them:

{
   "username": "xxxxx",
   "password": "xxxxx",
   "serverIpAddress": "xxx.xxx.xxx.xxx"
}

I hope that starts to make more sense. It might seem like a two step process, but in actual fact its a one step process when you consider that the developer does one thing (defines what settings are needed) and the user of the driver does another (creates the settings that are determined by the driver developer).

@neophob Thanks for the feedback. Those are definitely great points and I will have a think about them for sure and get back to you.

nklerk commented 6 years ago

I noticed that most developers are either having issues to implement proper device discovery or are unwilling to and end up with a edit this file solution. Then there are devices that can only be configured using multiple parameters.

I see two solutions:

I would prefer the later over the first. But I do realize the first would be easier and more likely to be fast deployed.

What I would see as a ultimate end goal are the following features:

For the user this brings the following benefits. Having access to verified stuff that crazy guys like us can come up with. The user won’t need any knowledge about code, installation and or Linux.

Shepless commented 6 years ago

Sorry - wrong button!

Shepless commented 6 years ago

@nklerk I've just put up a PR for review that might help with the hatdcoded issue you mentioned above #80

bdauria commented 6 years ago

I think we should try to keep the settings feature in a different thread, otherwise it's going to make it difficult to follow.

@Shepless You have some good points here, but to us it's important to provide a good developer experience, and some consistency across the different drivers created by the community. That's one of the reason why we came up with the CLI, and why we want to make it the default way of creating drivers. In my opinion, going in this direction is going to help us avoiding doing some support for developers that have less experience in manipulating low-level components (I understand that might not be your case) instead of the other way around (exposing lower-level features).

The community is an excellent way to grow our ecosystem, but we (NEEO) also need to drive the core of the SDK in regard to our internal strategy. This doesn't mean that developers can't take part in the discussions and contribute to it, but it's just guarantee that we don't allow everything coming in.

Now, creating some different packages to expose the low-level functionalities might make sense, but that's something we actually wanted to avoid (and that will probably become impossible as soon as the community drivers are able to run directly on the brain).

Shepless commented 6 years ago

@bdauria Thanks for the reply. I'm not sure if I've explained this too clearly so I'll try and address your feedback.

it's important to provide a good developer experience, and some consistency across the different drivers created by the community. That's one of the reason why we came up with the CLI, and why we want to make it the default way of creating drivers.

I agree 100% - good developer experience is critical. However, as a developer, I am a little concerned about the direction of the SDK, especially the flexibility and good developer experience it (currently) offers. If the CLI is intended as a scaffolding tool, then thats great as it lowers the barrier to entry to start building drivers. However, my worry here is the start server functionality and the loading of drivers from multiple locations (custom driver directory and nodemodules). IMO this actually reduces the consistency of drivers_ across the community and that in turn makes it harder for the more technically savvy developers to build the eco-system around it.

For example, more technical people will understand the benefits of building a single purpose driver package that exposes itself to be consumed and used in anyway:

const {buildDevice} = require('neeo-sdk');

module.exports = buildDevice('my Device') //...

But less technical people will leverage the CLI, which will allow them to create X number of drivers, in a non-fixed location and also start a server. As a member of the developer community trying to help build an eco-system - this makes it extremely difficult as drivers now have an inconsistent way in which they can be developed, published and exposed for consumption. In the real world with my driver manager it's just made the ability for me to help manage drivers a LOT more difficult. If I knew all drivers were consistently going to be just a driver that was exported and nothing more then I can easily write code to find them, install them and attach them to a NEEO server (that the driver manager controls). But now, a driver can potentially:

In the current stable SDK, we have consistency - every driver is self contained and starts its own server. For this reason, it has made the driver manger a breeze to create and working with the community NEEO drivers extremely easy. But with the @next SDK- the lack of consistency that will arise from this massively increases the complexity.

If you guys want a CLI to reduce the barriers to entry for less technical people, then I would suggest we take the route of create-react-app or ember-cli. Both of these CLIs are essentially wizards that ask the users questions:

The outcome of these questions basically creates your driver boilerplate. That's it. Then I would say, take out the ability to start a "production" server from the CLI and just start a server for development purposes - inline with what @nklerk was talking about above.

The community is an excellent way to grow our ecosystem, but we (NEEO) also need to drive the core of the SDK in regard to our internal strategy.

Again, I agree that the community is an important part of NEEO and it's why I'm enjoying it so much (because of these kind of discussions!). However, as contributors to the SDK and also to the surrounding eco-system it would be great if the internal strategy was roughly communicated so we are all on the same page and pulling in the same direction. It could be that all of this stiff I'm rambling on about is already defined in the internal strategy/roadmap - but I have no idea what that is as it's a black box from the outside looking in. Any kind of public milestones/roadmap will be of huge help here I think.

Now, creating some different packages to expose the low-level functionalities might make sense, but that's something we actually wanted to avoid (and that will probably become impossible as soon as the community drivers are able to run directly on the brain).

My apologies, I don't think I was too clear when I was talking about "low-level". What I was meaning is that the SDK is just for building drivers and nothing more. I called this "low-level" as from a non-technical users point of view, thats what it is - low level things they don't fully understand. I think this is perfectly fine - this is an SDK and not a user interface. I wasn't suggesting the SDK exposes "ultra low level" functionality, but just the ability to create drivers. This happy middle-ground allows you guys to focus on building the platform out (e.g. new UI widgets we can use etc.) so the community can focus more on the support of drivers and driver management. This greatly eases the burden on NEEO as the "sole providers" for any features that are required. But as I mentioned above, this doesn't mean NEEO can't adopt community efforts in-house.

I hope thats made a bit more sense - it's a hard thing to communicate effectively 😄

bdauria commented 6 years ago

@Shepless create-react-app and the angular-cli, have the goal to abstract away as much lower-level stuff (including configuration) as possible from the developer so that they can focus on their application, not just generating the required boilerplate. They also allow to "eject" to enable you to access the boilerplate and do some more advanced configuration and tweaking.

From what I remember, the eject functionality on angular-cli for instance wasn't available at the beginning but came later because the community requested it en masse. So going that direction might make sense for us at the beginning, until we really have to.

But in any case, we definitely need some more input on this because the needs can be very different. If there are a huge demand for starting the server manually, then we would need to expose the functionality. But again, the ultimate goal, also mentioned by @nklerk is to have the possibility to run the drivers directly on the Brain, so you never have to worry about starting the server by yourself.

What you mention regarding the consistency in providing driver from the community is interesting. To avoid this, we need to define some strong guidelines and maybe separate completely the driver publication from the runtime.

Regarding the public roadmap, it's not that we're trying to hide anything, but just that we haven't reached the point yet where we are able to define it clearly (the SDK is not our only concern ;) ). But we're definitely working on it and we will try to be as transparent as possible and involve the community as much as we can.

nklerk commented 6 years ago

@Shepless @bdauria I think we are all having the same goal and i trust NEEO fully to get there. The CLI is going to be a great step forward. As far as i can see it allows to start all drivers at once but also to start all drivers separately (multiple instances of the SDK-CLI) I think this is a good starting point for the driver manager. Let's be concrete here what we can do on a short timeframe. I suspect many difficult changes will follow along the way and lets not be scared of that ;-).

I want to propose the following for a short term solution:

I would suggest to @Shepless to only support CLI based drivers for now. This would require a repository that one of the community members takes care of. I'm thinking of a GitHub with a JSON file with some pre defined parameters like Name, Description, Icon, Developer, Git link, Planet Neeo link etc. The driver manager can check this online repository.

I'll make a GitHub including a concrete example of how i see a repository.

I would suggest NEEO to provide a stencil of how a driver file structure must look like to standardize things. An example is my Kodi driver, I have made it SDK-CLI ready but i had great difficulties to understand the best way to structure my GitHub files. i had many talks back and forth with @MichaelKohler about it and in the end requested if he could do a PM on my driver as i could just not understand how to do it right. I'm not on the same level as you guys all are and need a concrete example to follow. sorry for holding you guys back by that.

These actions will give us a solution right now while not restricting a future approach.

Shepless commented 6 years ago

@nklerk

sorry for holding you guys back by that.

On the contrary mate, you've been one of the main people driving the NEEO dev community forward! You're very highly valued and appreciated by myself and probably the whole NEEO team!

I would suggest to @Shepless to only support CLI based drivers for now.

This is my point above though - I'm not sure how I can do that very easily. In the current SDK a driver was just one driver, but in @next (and going forward) this is not true anymore. Someone can create a driver that depends and loads another driver, which in turn can depend and load another driver. This makes the driver manager pointless as it cannot control those processes and make it a stable environment (e.g. if a sub, driver dependency crashes - then the the parent driver also crashes).

Shepless commented 6 years ago

@bdauria @neophob so I have spent an hour creating an incredibly crude/poorly implemented example of how I saw the CLI - repo

pfiaux commented 6 years ago

I really like the ideas mentioned above, there's a lot of great points here, thanks for the thoughtful write ups @nklerk & @Shepless. That said there's almost too many points for me try to address it all in my reply, as @bdauria mentioned we should split up the discussion.

I've created #82 to discuss a short term roadmap for the SDK architecture. I'm really impressed by your suggestions @Shepless, at lot of it is inline what what I had previously discussed with @bdauria and some goes beyond it. Regarding #80 and the CLI repo (https://github.com/shepless/neeo-cli) I think both are a better fit for the discussion in #82. So bare with me as I move those topics out of the way, I'll mainly address the Driver Manager topic in my reply.

If you look at the multipleDevices or the discoverableLightDevice in the sdk examples you'll see they used multiple devices on a single server before the CLI came to be.

I think our motives behind the CLI are inline with those the Driver manager:

The CLI doesn't recursively load modules (or I don't think it does), it mainly starts a combination of local and node_modules packages (the main use case for local being development). So the driver manager could use it:

  1. Using the CLI the driver manager handles add/remove of packages in node_modules and restart of CLI, CLI then handles running the drivers (works with 1 server)
    • We can also imagine improving the CLI to allow easily running a single driver (say last parameter is the driver to run) then you can easily start one instance per driver.
  2. Without the CLI the driver manager implements it's own loading of drivers based on emerging standard we started in the CLI, it can then start them like the CLI either as separate or shared server/processes. (in this case it's more parallel to the CLI than on top of it)

By emerging standard i mean the main file exposing:

module.exports = {
  devices: [...devicesHere],
};

For discovery/registry of the modules I think as we're building on top of the npm eco system I would say prefixing with neeo-driver- should cover most of our short term needs and make the packages easy to find without the need for a repository.

pfiaux commented 6 years ago

This topic led to several other ones which are now done or in progress. I don't think we have any clear remaining goals or outcomes, I'll close but feel free to open up new issues for specific points that were not addressed here.