microsoft / BotFramework-Hubot

Hubot adapter for botframework
MIT License
111 stars 40 forks source link

Support Skype #31

Closed leonardochaia closed 5 years ago

leonardochaia commented 6 years ago

Should this provide support for Skype ?

I've created this repo I'm using for internal bots inspired on the MSTeamsMiddleware. Any chance we could extract the relevant logic of the MSTeamsMiddleware so it can be shared for Skype and MS Teams?

Thanks, Leo.

MatSFT commented 6 years ago

The adapter should work with skype without needing any of the teams specific things

leonardochaia commented 6 years ago

The adapter does seem to work, but TextMiddleware is being used by default, so Skype does not work properly (prepended bot name, "bot is typing", embedded images, and so on)

hubot_1  | [Fri Aug 24 2018 17:18:28 GMT+0000 (UTC)] DEBUG Executing listener callback for Message 'hubot ping'
hubot_1  | [Fri Aug 24 2018 17:18:41 GMT+0000 (UTC)] INFO hubot-botframework-adapter: onBotEvents
hubot_1  | [Fri Aug 24 2018 17:18:41 GMT+0000 (UTC)] INFO hubot-botframework-adapter: Handling activity Channel: skype; type: message
hubot_1  | [Fri Aug 24 2018 17:18:41 GMT+0000 (UTC)] INFO hubot-botframework-middleware: creating middleware...
hubot_1  | [Fri Aug 24 2018 17:18:41 GMT+0000 (UTC)] INFO hubot-botframework-middleware: TextMiddleware toReceivable

I've investigated my issue further on and I've got it to work by using the MSTeamsMiddleare as is, without modification, for skype channel with the following code:

{ registerMiddleware } = require('hubot-botframework').Middleware;
MicrosoftTeamsMiddleware = require('hubot-botframework').MicrosoftTeamsMiddleware;

registerMiddleware 'skype', MicrosoftTeamsMiddleware

Is this something that hubot-botframework could provide or is this on my side?

MatSFT commented 6 years ago

I think the appropriate way to handle this would be to copy the teams middleware into a new file for skype. I'm not sure that everything in teams applies to skype though.

MatSFT commented 6 years ago

feel free to submit a PR for this change

leonardochaia commented 6 years ago

Yep that's what I thought. I'll request a PR soon. Thanks.

maxpert commented 6 years ago

Looking at https://github.com/leonardochaia/hubot-botframework-skype is exactly the picture I imagine for different channels to add their own support.

leonardochaia commented 6 years ago

@maxpert said:

It would be really nice if we can keep these specifics away from core one of solutions in my head is to expose nicer hooks here and probably have a separate NPM package for teams all together. That will let you move faster (independent from such PRs) and keep this repo specific to hubot integration with botframework.

I do like where this is going. Channel specific logic would be kept apart of BotFramework-Hubot adapter's logic. In which case this repository would not be specific to any channel at all, which would allow channels repo to move faster if needed.

Maybe this discussion should be moved to a different issue.

MatSFT commented 6 years ago

I don't actually like the method of splitting the botframework adapter's middleware into separate packages. Its unnecessary (not like we have that much code anyway) and makes it harder to use for the user.

@maxpert you designed this system to handle multiple channels at the same time and thats where it can excell at... as a user it would be really really annoying to have to (1) install botframework adapter... (2) run the thing... (3) find out its not working in skype or teams... (4) realize theres yet another set of packages I have to install and update.

From an engineering standpoint, this separation makes alot of sense. From a usage standpoint, I think we should just include all the packages in one.

maxpert commented 6 years ago

Well I can play devil's advocate here and I will do so 😄. Hubot probably already has adapters for slack, telegram etc. I am not sure how many of people will be interested in using hubot + slack with botframework. What I am sure is the fact that MS Teams has only entry point via us, and so does Skype; we should be supporting them as a first party citizens.

leonardochaia commented 6 years ago

It makes sense to me that Microsoft/BotFramework-Hubot provides official support for Microsoft channels.

Under that statement, I'll request a PR with a middleware for Skype based upon the MS Teams middleware, containing only what is necessary for Skype.

maxpert commented 6 years ago

Agreed; if they can work from same adapter then it's just a single line.