RPC API miner_getstathr: Add "ispaused" information into response. #1232
Conversation
libapicore/ApiServer.cpp
Outdated
| @@ -423,6 +424,13 @@ Json::Value ApiConnection::getMinerStatHR() | |||
| gpuIndex++; | |||
| } | |||
|
|
|||
| gpuIndex = 0; | |||
There was a problem hiding this comment.
Instead of having 3 different for loops (inlcuding this last yours) can we manage to pull info from all gpus using only 1 for loop ?
There was a problem hiding this comment.
Also ... ispaused is a little too generic.
Maybe we could add some info about the pausing reason ? (eg. temp threshold )
There was a problem hiding this comment.
loops 👍
ispaused: I already thought about that and came due following reasons to the current solution:
- as I implemented -tstop/-tstart I already thought about a pause bit flag (TEMPERATURE, USER, ...)
but currently we have only pausing due temperature reason (no keypress and no api to pause mining) - ispaused is a bool, if there will be more reasons I thought the API can respond a "pausereasons" or so. I've noticed content strings changes more often than bools ;-) which always hurts in interfaces.
So I have following questions before adapting PR:
- ispaused: bool or string
- If ispaused is a string and if not paused: null or "" or "no" or "0" ?
- If ispaused is a string and if multiple reasons for pausing pending - join reasons with "," or ";"
- Should I prepare pausing to be a unsigned int with bits and adapt is_mining_paused() ?
There was a problem hiding this comment.
I'll try explain what I would like to achieve.
At present the result of getstathr is like this
{
"id":0,
"jsonrpc":"2.0",
"result":{
"ethhashrate":87388615,
"ethhashrates":[
14506510,
14593898,
14593898,
14593898,
14506510,
14593898
],
"ethinvalid":0,
"ethpoolsw":0,
"ethrejected":0,
"ethshares":136,
"fanpercentages":[
90,
90,
90,
90,
90,
90
],
"pooladdrs":"10.0.0.113:5551",
"powerusages":[
0.0,
0.0,
0.0,
0.0,
0.0,
0.0
],
"runtime":"112",
"temperatures":[
54,
51,
57,
59,
62,
60
],
"version":"0.15.0.dev11-96+commit.6b02ef2f"
}
}
I whish we could manage gpu's as an array of objects (instead of parsing a string which has to be split into an array of values)
{
"id":0,
"jsonrpc":"2.0",
"result":{
"hashrate":87388615,
"shares":136,
"invalid":0,
"rejected":0,
"poolsw":0,
"pooladdrs":"10.0.0.113:5551",
"runtime":"112",
"version":"0.15.0.dev11-96+commit.6b02ef2f.dirty",
"gpus" : [
{ "index" : 0,
"hashrate : 14506510,
"power" : 0.0,
"fan" : 90,
"temp" : 54,
"temp-stop" : 0,
"temp-start" : 0,
"status" : "<operational|paused|error>",
"reason" : "<null|at-temp|manual|error>"
},
{ "index" : 1,
"hashrate : 14506510,
"power" : 0.0,
"fan" : 90,
"temp" : 54,
"temp-stop" : 0,
"temp-start" : 0,
"status" : "<operational|paused|error>",
"reason" : "<null|at-temp|manual|error>"
},
{ "index" : 2,
"hashrate : 14506510,
"power" : 0.0,
"fan" : 90,
"temp" : 54
"temp-stop" : 0,
"temp-start" : 0,
"status" : "<operational|paused|error>",
"reason" : "<null|at-temp|manual|error>"
},
{ "index" : 3,
"hashrate : 14506510,
"power" : 0.0,
"fan" : 90,
"temp" : 54
"temp-stop" : 0,
"temp-start" : 0,
"status" : "<operational|paused|error>",
"reason" : "<null|at-temp|manual|error>"
},
{ "index" : 4,
"hashrate : 14506510,
"power" : 0.0,
"fan" : 90,
"temp" : 54
"temp-stop" : 0,
"temp-start" : 0,
"status" : "<operational|paused|error>",
"reason" : "<null|at-temp|manual|error>"
},
{ "index" : 5,
"hashrate : 14506510,
"power" : 0.0,
"fan" : 90,
"temp" : 54
"temp-stop" : 0,
"temp-start" : 0,
"status" : "<operational|paused|error>",
"reason" : "<null|at-temp|manual|error>"
}
]
}
}
Would not set bit-shifting values for reasons of paused as this would imply consumers of api to have a legend of values and also bit shifting may be affected by endianess.
I know this scheme is a bit wordy but is more Json style than the actual.
Obviously this is my view, not necessarily the same of other devs. Thus some more feedback may be needed.
P.s. Anticipating your question about compatibility with ... erm ... other miners : getstathr is ethminer's only ... so we can change almost everything leaving gestat1 as is.
There was a problem hiding this comment.
In this way we could also add each gpu object any property we may need (ex. Vendor, Compute level and so on) changing only the structure of the GPU object while leaving the "result" structure intact.
There was a problem hiding this comment.
Would not set bit-shifting values for reasons of paused as this would imply consumers of api to have a legend of values and also bit shifting may be affected by endianess.
I just thought preparing internal a flag for pausing (replace 'm_wait_for_tstart_temp' with 'm_mining_paused_flag' and adding MINIG_PAUSED_FLAG_WAIT_TSTART_TEMP=0x01, MINIG_PAUSED_FLAG_MANUAL = 0x02, MINIG_PAUSED_FLAG_ERROR = 0x04, ...)
==> In this case there could be more than one reason we have to return.
P.s. Anticipating your question about compatibility with ... erm ... other miners : getstathr is ethminer's only ... so we can change almost everything leaving gestat1 as is.
Yes I know - In miner_getstathr I feel free to change nearly everything ;-) That was the reason I added the new info to miner_getstathr and not to miner_getstat1.
Obviously this is my view, not necessarily the same of other devs. Thus some more feedback may be needed.
Yep - I'll wait for more reviews...
|
Why aren't we moving towards a more RESTful approach. Modern choices seem to be between REST and SOAP these days. http://miner1/status - would return the global properties of the miner. Things like miner name, software version, # of GPUs, etc... (using JSON encoding in a websocket wrapper). http://miner1/gpu3/status - would return GPU specific attributes for GPU 3 (for example). ie. power, temp., fan speed, etc... Action commands would be implemented as follows: http://miner1/shutdown http://miner1/gpu2/disable Bonus free support: Authentication is built-in to http protocol. You get the idea... |
|
While SOAP is a protocol and REST is an architectural style (thus not very comparable) the main issue I see using http transport is it's stateless: every request have to pass through all authentication stack (if any) and in case of complex operations may involve the need to cache session state. It' a huge overhead not count the continuos opening and closing of sockets. |
|
I guess we'll just have to agree to disagree. I think providing a standard approach to secure remote management that takes advantage of HTTP transport features like authentication and security, outweighs any argument that can be made against stateless ephemeral connections. The days of worrying about network overhead for HTTP are in the past. Even the tiniest devices implement full network stacks (including encryption) these days, without breaking a sweat. I find the notion of authentication on a statefull unencrypted channel a just a little bizarre. There is no way I'd ever let a router/firewall allow an unencrypted miner connection to be initiated from outside my local network. |
|
I think the problem should be split in two. Mining operations where monitoring is made inside the local network and remote access to single rig. In first case I do not know anyone (but my contacts may be limited) who would like to drop socket/json format (it's way easier to implement and way faster) while in the latter I may agree with you that for hobbyst/small operations (max 4 to 5 rigs) a higher level of security may be appreciated if you plan to expose ethminer on the web. |
|
Documentation is another matter. We could make a mess even using REST if we do not document well both the entry points and payloads (both incoming and outgoing). |
It depends on how sensible are the informations which could be pulled from that connection. |
There was a problem hiding this comment.
Approved... Not directly related but I really think the structured JSON response @AndreaLanfranchi presented is superior to the present situation. Perhaps a new, not backward compatible, API query? It would be simpler to traverse programmatically. Could be done under a separate PR.
|
@AndreaLanfranchi Are you intending to release this PR? It's not clear what specific changes you are requesting, and a complete rewrite or reformat really belongs in a separate PR. Is it the combination of loops you're waiting for? |
Yes ... but if needed I can withdraw my comment immediately. |
StefanOberhumer
force-pushed
the
api-add-ispaused
branch
from
3d02c65 to
6df515e
Compare
|
|
Please also add new member in API_DOCUMENTATION |
Adding possibility to check ispaused if hashrate on a card drops to 0.