Skip to content
This repository has been archived by the owner on Feb 16, 2020. It is now read-only.

listening to portfolioUpdate does not work #1541

Closed
saveriocastellano opened this issue Dec 23, 2017 · 7 comments
Closed

listening to portfolioUpdate does not work #1541

saveriocastellano opened this issue Dec 23, 2017 · 7 comments

Comments

@saveriocastellano
Copy link

saveriocastellano commented Dec 23, 2017

Note: for support questions, please join our Discord server

  • I'm submitting a ...
    [x ] bug report
    [ ] feature request
    [ ] question about the decisions made in the repository

  • Action taken (what you did)

hello, I'm trying to listen to "portfolioUpdate" event in a plugin.. but with no success..
In my plugin I have defined two methods, one "portfolioUpdate" and the other one "processPortfolioUpdate". Non of these methods ever gets called :(

I have seen that the IFTTT plugin defines this method:

IFTTT.prototype.portfolioUpdate = function(portfolio) {
var message = "Gekko has detected a portfolio update. " +
"Your current " + config.watch.currency + " balance is " + portfolio.currency + '.' +
"Your current " + config.watch.exchange + " balance is " + portfolio.assert + '.';
this.send(message);
}

I guess this is also never called, as I used the same code in my plugin.

It is not clear to me how the other 'process' like events are registered by plugins (it seems that this is done automatically when the corresponding methods are defined and found).
In plugins/trader/trader.js I found the following:

this.manager.once('portfolioUpdate', portfolioUpdate => {
this.initialPortfolio = portfolioUpdate;
})

So it seems to be that the "portfolioUpdate" event is not fired, so I tried adding this:

this.manager.on('portfolioUpdate', portfolioUpdate => {
this.emit('portfolioUpdate', portfolioUpdate);
})

but still, the "portfolioUpdate" and "processPortfolioUpdate" methods in my plugin do not get called.
How can a plugin listen to such event? Does it need to explicitely register itself as a listener by calling the "on" or "once" methods of "portfolioManager" ? How can this be done? How can a plugin directly call the portfolioManager? I see that the portfolioManager is created by the treader, so i don't think it must be directly accessed by other plugins.. can you explain ?

  • Expected result (what you hoped would happen)

  • Actual result (unexpected outcome)

  • Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. stackoverflow, etc)

@saveriocastellano saveriocastellano changed the title listening to portfolioUpdate does not workk listening to portfolioUpdate does not work Dec 23, 2017
@cmroche
Copy link
Contributor

cmroche commented Dec 24, 2017

There doesn't seem to be an entry for this plugin in subscriptions.js, adding it there and possibly this call util.makeEventEmitter(Base); ought to fix the issue.

@saveriocastellano
Copy link
Author

thanks for your reply. In subscriptions.js I see this:

{
emitter: ['trader', 'paperTrader'],
event: 'trade',
handler: 'processTrade'
},
{
emitter: ['trader', 'paperTrader'],
event: 'portfolioUpdate',
handler: 'processPortfolioUpdate'
}

doesn't this mean that plugins can listen to 'trade' and 'portfolioUpdate' events by defining 'processTrade' and 'processPortfolioUpdate' methods?

This is exactly what I have done, but still it doesn't work...

@cmroche
Copy link
Contributor

cmroche commented Dec 24, 2017

@saveriocastellano Ok I checked my past work, I got it all backwards. Those files are to register the emitters, which is correct in this case. Technically as long as IFTTT has the processPortfolioUpdate I think it should work.

@saveriocastellano
Copy link
Author

but I'm telling you... the entry is there, and it doesn't work

@saveriocastellano
Copy link
Author

@cmroche ok finally I have figured out what the problem with "processPortfolioUpdate" is..
it is basically due to the initialization flow (loading plugins, registering plugin handlers, starting to watch the market): what happens is that portfolioManager gets the balance from the exchange BEFORE plugins handlers are registered... this results in the "portfolioUpdate" event with the initial balance to be fired BEFORE any "processPortfolioUpdate" method from other plugins is registered as a handler for this event.

Looking at the code at the top of trader.js I saw this:

this.manager.on('trade', trade => {

if(!this.sendPortfolio && this.initialPortfolio) {
  this.emit('portfolioUpdate', this.initialPortfolio);
  this.sendPortfolio = true;
}
this.emit('trade', trade);

});

I believe this is there to notify plugins about the initial balance when the first trade takes place. Problem is that this is not so useful, because trades happen rarely and thus it is not good for plugins to not be aware of the actual balance until the first trade takes place... As as work around I added similar code to the "processCandle" method in trader.js :

Trader.prototype.processCandle = function(candle, done) {
if(!this.sendPortfolio && this.initialPortfolio) {
this.emit('portfolioUpdate', this.initialPortfolio);
this.sendPortfolio = true;
}
done();
}

with this code, the balance is notified to plugins as soon as the first candle is obtained.. this is early enough. This is probably not the best/cleanest solution but it works, and I could not find anything better... let me know what you think

@werkkrew
Copy link
Contributor

werkkrew commented Jan 4, 2018

Looking for a more elegant solution to this.

@askmike
Copy link
Owner

askmike commented Feb 3, 2018

@saveriocastellano @cmroche apologies for the late reply.

The gist of the problem is that there are 2 event systems in the current Gekko, one for internal use (plugins) and one to be broadcasted upstream (via parent process to the web UI). I've just started to update the first system and remove the second one. Keep track of that here: #1850.

The current WIP branch includes this updated event. I also encourage to check out the new documentation which hopefully makes things more clear: https://github.com/askmike/gekko/blob/feature/sync-gekko-events/docs/internals/events.md

I'm closing this so we can keep all discussion in #1850.

@askmike askmike closed this as completed Feb 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants