Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions catkit2/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,7 @@ PYBIND11_MODULE(catkit_bindings, m)
.def("interrupt_service", &TestbedProxy::InterruptService)
.def("terminate_service", &TestbedProxy::TerminateService)
.def("shut_down", &TestbedProxy::ShutDown)
.def("reload_config", &TestbedProxy::ReloadConfig)
.def_property_readonly("message_broker", &TestbedProxy::GetMessageBroker)
.def_property_readonly("is_simulated", &TestbedProxy::IsSimulated)
.def_property_readonly("is_alive", &TestbedProxy::IsAlive)
Expand Down
72 changes: 72 additions & 0 deletions catkit2/testbed/testbed.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ def __init__(self, port, is_simulated, config):
self.server.register_request_handler('get_service_info', self.on_get_service_info)
self.server.register_request_handler('register_service', self.on_register_service)
self.server.register_request_handler('shut_down', self.on_shut_down)
self.server.register_request_handler('reload_config', self.on_reload_config)

self.is_running = False
self.shutdown_requested = threading.Event()
Expand Down Expand Up @@ -668,6 +669,16 @@ def on_register_service(self, data):

return reply.SerializeToString()

def on_reload_config(self, data):
request = testbed_proto.ReloadConfigRequest()
request.ParseFromString(data)

new_config = json.loads(request.config)
self.reload_config(new_config)

reply = testbed_proto.ReloadConfigReply()
return reply.SerializeToString()

def on_shut_down(self, data):
self.shutdown_requested.set()

Expand All @@ -686,6 +697,67 @@ def register_service_type(self, service_type, path):
'''
self.service_type_paths[service_type] = path

def reload_config(self, new_config):
'''Reload the testbed configuration.

This updates the configuration without restarting the testbed.
Services must be restarted to pick up their new configuration.

Parameters
----------
new_config : dict
The new configuration dictionary.
'''
self.log.info('Reloading configuration...')

# Update the config
old_config = self.config
self.config = new_config

# Update simulation mode if it changed
if 'testbed' in new_config and 'simulated' in new_config['testbed']:
self.is_simulated = new_config['testbed']['simulated']

# Check for added/removed services

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're just checking new/removed services, but not updating the dependency graph. A correct dependency graph is required for correct shutdown of the testbed.

old_services = set(old_config.get('services', {}).keys())
new_services = set(new_config.get('services', {}).keys())

added = new_services - old_services
removed = old_services - new_services

if added:
self.log.info(f'New services in config: {list(added)}')
# Add new service references
for service_id in added:
service_info = new_config['services'][service_id]
service_type = service_info['service_type']

if self.is_simulated and 'simulated_service_type' in service_info:
service_type = service_info['simulated_service_type']

dependencies = service_info.get('depends_on', [])
self.services[service_id] = ServiceReference(service_id, service_type, ServiceState.CLOSED, dependencies, self.message_broker)

if removed:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could also have a warning for services that are running of which the config has changed?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have been using this mechanism to update flat maps, tip tilt stage position defaults, camera offset_x, offset_y defaults in case we want to start / stop the services and have them come up the the new default without restarting the testbed. Since this is all tracked in the services.yml config file I can track it on git. Generally speaking when I hit reload I want to make sure the new configs are in place. A warning may over alert the user. Maybe info level would be a good way to status what has changed. I could go either way though

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed on INFO. I meant "warning" as in "some kind of log message" rather than "a warning log message". I could've been more clear on that.

self.log.warning(f'Services removed from config: {list(removed)}')
# Check if any removed services are running
for service_id in removed:
if service_id in self.services and self.services[service_id].is_alive:
self.log.warning(f'Service "{service_id}" is running but removed from config. Consider stopping it.')

# Update startup services list
self.startup_services = []
if 'safety' in self.config.get('testbed', {}):
self.startup_services.append(self.config['testbed']['safety']['service_id'])

if 'startup_services' in self.config.get('testbed', {}):
self.startup_services.extend(self.config['testbed']['startup_services'])

if self.is_simulated and 'simulator' not in self.startup_services:
self.startup_services.append('simulator')

self.log.info('Configuration reloaded successfully.')

def start_service(self, service_id):
'''Start a service.

Expand Down
20 changes: 20 additions & 0 deletions catkit_core/TestbedProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,26 @@ void TestbedProxy::ShutDown()
}
}

void TestbedProxy::ReloadConfig(const json &new_config)
{
catkit_proto::testbed::ReloadConfigRequest request;
request.set_config(new_config.dump());

catkit_proto::testbed::ReloadConfigReply reply;

try
{
reply.ParseFromString(MakeRequest("reload_config", Serialize(request)));
}
catch (...)
{
throw std::runtime_error("Unable to reload config.");
}

// Invalidate cached config so it gets refetched on next access
m_HasGottenInfo = false;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable is just for this instance of a TestbedProxy. It won't affect other TestbedProxy objects. I think you're aware of this, but I wanted to make sure.

}

std::shared_ptr<DataStream> TestbedProxy::GetHeartbeat()
{
GetTestbedInfo();
Expand Down
1 change: 1 addition & 0 deletions catkit_core/TestbedProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class TestbedProxy : public Client, public std::enable_shared_from_this<TestbedP
bool IsAlive();

void ShutDown();
void ReloadConfig(const nlohmann::json &new_config);

std::shared_ptr<DataStream> GetHeartbeat();
std::shared_ptr<MessageBroker> GetMessageBroker();
Expand Down
51 changes: 51 additions & 0 deletions proto/service.proto

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file might've been committed accidentally?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops! thanks for catching that

Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,57 @@ import "core.proto";

package catkit_proto.service;

message GetInfoRequest
{
}

message GetInfoReply
{
string service_id = 1;
string service_type = 2;
string config = 3;

repeated string property_names = 4;
repeated string command_names = 5;
map<string, string> datastream_ids = 6;

map<string, string> property_datastream_links = 8;

string heartbeat_stream_id = 7;
}

message GetPropertyRequest
{
string property_name = 1;
}

message GetPropertyReply
{
Value property_value = 1;
}

message SetPropertyRequest
{
string property_name = 1;
Value property_value = 2;
}

message SetPropertyReply
{
Value property_value = 1;
}

message ExecuteCommandRequest
{
string command_name = 1;
Dict arguments = 2;
}

message ExecuteCommandReply
{
Value result = 1;
}

message ShutDownRequest
{
}
Expand Down
9 changes: 9 additions & 0 deletions proto/testbed.proto
Original file line number Diff line number Diff line change
Expand Up @@ -120,3 +120,12 @@ message ShutDownRequest
message ShutDownReply
{
}

message ReloadConfigRequest
{
string config = 1;
}

message ReloadConfigReply
{
}
Loading