Added clientinfo command support #16
Conversation
server_cmds.go
Outdated
| // Takes the form of "/channelID" | ||
| DefaultChannel string `ms:"client_default_channel"` | ||
|
|
||
| //TODO: I never managed to receive any value for this field |
There was a problem hiding this comment.
little niggle could we format it like:
// TODO(Henner25): ....
server_cmds.go
Outdated
|
|
||
| // Milliseconds since the client did something, for example sending | ||
| // a message, muting themselves or talking. | ||
| IdleTime int64 `ms:"client_idle_time"` |
There was a problem hiding this comment.
Could we capture this as a time.Duration, so we don't have to know its in Milliseconds?
server_cmds.go
Outdated
| Servergroups int `ms:"client_servergroups"` | ||
|
|
||
| // UTC timestamp at which the client has been created. | ||
| Created int64 `ms:"client_created"` |
There was a problem hiding this comment.
Could we do this a time.Time, same for other time based fields?
This also adds a proper struct to hold the response data. Documentation of the fields is a mix of the server query manual and experiments with an actual server. I could not figure out the meaning or data type for all fields, most notably `client_badges` and `client_meta_data` which I even excluded from the struct definition. Data for the tests is the incomplete data from the manual supplemented by real query results. Fields like ip address and UUIDs have been anonymized by replacing characters. I tried not to introduce characters which were previously not used in the value.
|
Ah, you're quick to respond. I saw the lints and fixed them by amending and force-pushing the commit. TODO's will be tagged with my name. I'd have loved to the time package types from the beginning. However the mapstructure decoder needs to know to consider the integer values to be milliseconds and utc timestamps and I wanted to use |
|
It should be pretty trivial to add the time.Duration decoder see: |
|
The target field needs some way to declare the duration units since some data is returned as seconds (e.g. I'm thinking about specifying the unit in the struct tag itself; I think that makes sense since is basically a serialization/representation issue, too. Do either of those sound sensible to you or am I missing something about the unit-issue? |
|
I like the idea of adding a tag, that seems like a nice clean solution 👍 |
|
I think it really won't work :( It seems that the mapstructure DecoderHooks cannot access the tags of a struct field anymore: After finding the correct struct field to decode into, the recursive decode call passes I'll implement my alternative suggestion and see how nicely that turns out. |
|
Really not keen on the alternative method will have a think tonight and see
if I can come up with an alternative solution
…On Tue, 3 Jul 2018 at 17:36, Henner25 ***@***.***> wrote:
I think it really won't work :( It seems that the mapstructure
DecoderHooks cannot access the tags of a struct field anymore: After
finding the correct struct field to decode into, the recursive decode call
<https://github.com/mitchellh/mapstructure/blob/master/mapstructure.go#L1013>
passes reflect.Value of the struct field but we'd need the
reflect.StructField type to access the tags. All we have in the decoder
is now the "time.Duration" value without knowing which (if any at all!)
struct it came from.
I'll implement my alternative suggestion and see how nicely that turns out.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#16 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAGXL35Otau4XaUpa7SlPGEb0AYTssrXks5uC52WgaJpZM4U-RNg>
.
|
|
Having looked at this the simplest solution may be to use wrapping struct which has a reference to the type being decoded so it can gain context. This will still fall short if the struct requires two different decode methods e.g. one ms based duration and one second based duration as it still won't be able to tell which field without improving the hook system :( |
This fixes the Servergroups field of DetailedOnlineClient. Credits for the discovery and solution go to irgendwr.
| // sliceHookFunc supports int/string decoding to slice | ||
| func sliceHookFunc(from reflect.Kind, to reflect.Kind, data interface{}) (interface{}, error) { | ||
| if to == reflect.Slice { | ||
| if from == reflect.String { |
There was a problem hiding this comment.
Both of these if's cant be true so a switch on from would be better.
Added support for the clientinfo command which returns detailed information about individual online clients.
The client id
clidis not included in the server response, probably because it was used to request the data in the first place, but I think it is useful to include it in the struct.Naming scheme
I dropped the "client_" prefix from the response fields and camelcased the rest, for example
client_talk_power -> TalkPowerbutconnection_packets_sent_total -> ConnectionPacketsSentTotal.Notes about specific fields of the server response
These information are also included as comments at the field definitions.
client_meta_dataandclient_badges: They are currently commented out in the struct definition. Because of insufficient data I could not infer a proper type for them and I do not want to risk a later API change because the type I picked was wrong.client_is_talker: The manual states that this reflects whether the client has enough talk power to talk, however this was always 0 during testing.client_lastconnected: According to the manualBut the actual values I receive corresponded to the timestamp of me being online for the last time, that is the time just before I disconnected from the server.
Test
The mock-response for the test is a mix of real data and the example from the manual because the latter consists of very few fields only. I scrambled possibly identifying information (IP-addresses, UIDs etc.), hopefully without introducing illegal characters.
N.B.
The first commit adds the
ChannelIDfield to theOnlineClientstruct. I think there was a mixup betweenclidandcid.