Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hashrate printed per card. #225

Merged
merged 4 commits into from Aug 16, 2017

Conversation

Projects
None yet
7 participants
@evilny0
Copy link
Contributor

commented Aug 5, 2017

I added hashrate per card, and colors for accepted/refused shares.

@evilny0 evilny0 referenced this pull request Aug 6, 2017

Closed

Improved console output #164

@chfast
Copy link
Contributor

left a comment

Looks good, but small changes are required.

p_farm->acceptedSolution(m_stale);
}
else {
cwarn << ":-( Not accepted.";
cwarn << EthRed << ":-( Not accepted.";

This comment has been minimized.

Copy link
@chfast

chfast Aug 8, 2017

Contributor

cwarn is red by default.

{
mh = _p.minerRate(i) / 1000000.0f;
sprintf(mhs, "%.2f", mh);
_out << " GPU" << gpuIndex++ << ": " << std::string(mhs) + "MH/s";

This comment has been minimized.

Copy link
@chfast

chfast Aug 8, 2017

Contributor

Instead of using sprintf you can do:

<< std::fixed << std::setprecision(2) << mh << "MH/s";
};

inline std::ostream& operator<<(std::ostream& _out, WorkingProgress _p)
{
float mh = _p.rate() / 1000000.0f;
char mhs[16];
sprintf(mhs, "%.2f", mh);
_out << std::string(mhs) + "MH/s";
_out << "Total: " << std::string(mhs) + "MH/s";

This comment has been minimized.

Copy link
@chfast

chfast Aug 8, 2017

Contributor

I'd prefer more condensed output. Maybe:

123.43 MH/s (5.13 54.12 12.00 8.01)
@evilny0

This comment has been minimized.

Copy link
Contributor Author

commented Aug 8, 2017

cwarn has a red background on the date, that's right :).

I condensed the output, but left the GPU ID : I think it's better when we want to tweak the GPU settings to see the ID. I put in blue, in white it was not different enough from the hashrates.

@chfast

chfast approved these changes Aug 10, 2017

@Blacksuu

This comment has been minimized.

Copy link

commented Aug 15, 2017

i was grab https://ci.appveyor.com/project/ethereum-mining/ethminer/build/307 to check "Properly reset text color after accepted shares." & for win 7 noo color, but i see build was not pass on 1 check

@evilny0

This comment has been minimized.

Copy link
Contributor Author

commented Aug 15, 2017

Yes. All circleci checks have been failing for a few days, no idea why, I don't even know what it's checking :).

For colors on windows, you need to have a look at #226.

@Blacksuu

This comment has been minimized.

Copy link

commented Aug 15, 2017

If you say "For colors on windows, you need to have a look at #226" then this change is for what ? only "Hashrate printed per card" what work on what S.O. & at what it refer then "Properly reset text color after accepted shares." ?

@evilny0

This comment has been minimized.

Copy link
Contributor Author

commented Aug 15, 2017

One of the changes in this PR is to add green color to accepted shares line so it stands out. I forgot to reset it at the end of the line, so I added a new commit to fix it.

However, colors only work on ANSI terminals, so if you want color on windows terminal which is not ANSI, you need #226.

If you want both (hashrate per card + color on windows terminal), then you'll need to take both changes and compile yourself.

@Blacksuu

This comment has been minimized.

Copy link

commented Aug 15, 2017

k then if "green color to accepted shares" well that green color is not there at all & about that was my feedback & now when you was say exactly were i can confirm 100% is noo green color

@evilny0

This comment has been minimized.

Copy link
Contributor Author

commented Aug 15, 2017

Let me rephrase : colors are not supported on windows terminal with the current code, including #225. It only works on Linux.

I added support for colors in windows terminal in #226. You can take the binary from #226 to have colors.

@Blacksuu

This comment has been minimized.

Copy link

commented Aug 15, 2017

dear one when i ask about what S.O. it is supose to work this P.R. it was about what Sistem operation aka windows 8,7,10, linux, unix
PR say: "I added hashrate per card, and colors for accepted/refused shares."

  1. hashrate per card = check it working = OK
  2. colors for accepted/refused shares (green) = NOt working = NOt functional ....
    Now if point 2 is working only for linux or only for win10 or only for unix (S.O.) say it & it will save for bouth of us losing time
@evilny0

This comment has been minimized.

Copy link
Contributor Author

commented Aug 15, 2017

I've been patient enough, and you are definitely right I'll stop losing my time after this post.

I already said twice colors are not supported on windows, unless you take #226. And I mean all versions of Microsoft Windows, including Windows 7 and Windows 10. I don't know how to phrase it so it will be more clear to you, sorry.

@Blacksuu

This comment has been minimized.

Copy link

commented Aug 15, 2017

let me clarifies few think first you came & make some change / improvement (for what all & i am happy) what suppose to be good or bad & ppls like me or other to give you a feedback
if you are not clear in what you PR it make it will create confusion & moment like this
you was add a PR were you was bring full color & hrate what was not work for all S.O. i was get it & we discuss that, after you decide to split PR in 2 one for Hrate / card & 1 for Color what was perfect undestenble ...
Color PR it was clear you was try to make work for windows & i undestend that from start i can read ...
Printed Hrate / card i was undestend it work & i was not say nothing till i seen it have color to BUT if you will show me were you was say from begining this PR is only for linux then mea culpa i was blind ...

PR title Hashrate printed per card PR description I added hashrate per card, and colors for accepted/refused shares. aka is NO specifie any SO & if was say all this was avoid & noo confusion created ...
So i am glad for any improve what you doo & all doo for make this miner better coz is realy good one

@AndreaLanfranchi

This comment has been minimized.

Copy link
Collaborator

commented Aug 16, 2017

Sorry if i drop in uninvited but ... I really do not understand this discussion about colors: IMHO colors are useful for those who like staring at their monitor continuosly while jumping on the seat for every share successfully submitted (probably shouting "Yuppie!"). On a headless mining rig this "feature" is completely unnecessary.
I think this mining program is great : with a quality well beyond others and ... the code is here to be read and checked ! Let's concentrate to make it better and, above all, a big thank you all contributors.

@chfast

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2017

CircleCI failures are not related. I'll have to fix it.

@chfast chfast merged commit 168e3f7 into ethereum-mining:master Aug 16, 2017

2 of 3 checks passed

ci/circleci Your tests failed on CircleCI
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@aliceaia

This comment has been minimized.

Copy link

commented on 33ba5d8 Aug 24, 2017

Color for found shares not work. Sorry for my bad english

This comment has been minimized.

Copy link
Collaborator

replied Aug 24, 2017

are you on linux or windows? (windows does not work)

@ShawnMcGough

This comment has been minimized.

Copy link

commented Aug 24, 2017

Thank you for leaving the GPU ID. Combined with the ethminer -G --list-devices command, it is very useful as I swap out cards to flash bios and see improvements.

Everything looks good and working well on linux. Thanks for the effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.