Repository navigation
Add ListMeasurementTypes to timetagger protos - #15
ziyuguo716 wants to merge 3 commits into
Conversation
Signed-off-by: Ziyu Guo <guoziyu0716@hotmail.com>
Signed-off-by: Ziyu Guo <guoziyu0716@hotmail.com>
1be28e0 to
147974f
Compare
| // Timetagger adapter is capable of reaching multiple devices while other adapters do not. | ||
| // Network address (host:port) of the specific Swabian device to target. | ||
| // Only meaningful for checking Swabian hardware devices health | ||
| optional string device_host = 3; |
There was a problem hiding this comment.
Adapter can get the device_host based on the product_id. I dont think we need to add the device_host here.
There was a problem hiding this comment.
As I indicated in the comment L41, this is just a temporary solution to ensure backward compatibility. Once the hardware registry is in-place, this field can be removed.
And all the adapters are currently unable to get device_host by product_id. device_host is stored in postgres which only the controller has access to by the existing architecture design.
| TIME_TAGGER_STATE_DEGRADED = 7; | ||
| } | ||
|
|
||
| enum ResourceType { |
There was a problem hiding this comment.
We should not have any vendor specific details in the proto - RESOURCE_TYPE_SWABIAN.
We can include it in the testdata/ folder and use for verification.
There was a problem hiding this comment.
Sounds good. Will remove vendor specific details
| ResourceType resource_type = 1; | ||
| } | ||
|
|
||
| message MeasurementTypeInfo { |
There was a problem hiding this comment.
We only need id, description, name here.
As a HAL prc, we are just giving the adapter owner to expose what measurements their adapter supports, so that the controller can invoke those measurments.
The rest of the fields are very specific to our timetagger adapter and should not be here. Our adpater can be modified to work with the HAL rpc defined here, not the other way round.
There was a problem hiding this comment.
Also, the way our controller handles this currently it sends the measurement name and assumes that the measurement is present and implemented on timetagger. If we have a third party adapter, it may not be right to assume that.
Maybe we add a test in this repo to verify the measurements id/name returned by the adapter when ListMeasurementTypes is invoked conatins whatever measurements the controller expects. It could have additiona measurements, that the controller does not expect as well, but should have everything the controller expects, so if the controller is invoked with that adapater, it would work.
ListMeasurementTypesrpc back to timetagger rpcdevice_hostas optional field inResourceHealthCheckRequest,StartDataCollectionRequest,StopDataCollectionRequestso that timetagger adapter retains its ability to reach multiple devicesv0.0.2