Composition of specifications and devices #13

Open
kolpakov.mm wants to merge 27 commits from dev-maxim into dev
Member

Implemented builders for PropertyDescriptor and ActionDescriptor. Some typos have been fixed. Introduced a new hierarchical device structure based on CompositeDeviceSpec and ConfigurableCompositeDevice. They draw ideas from DeviceSpec and DeviceBySpec. This is a prototype implementation, and further refactoring is expected. The key feature is clear composition of device specifications for a declarative description of devices and their behavior. Moreover, the API for parent and child devices (specs) is the same.

Implemented builders for PropertyDescriptor and ActionDescriptor. Some typos have been fixed. Introduced a new hierarchical device structure based on CompositeDeviceSpec and ConfigurableCompositeDevice. They draw ideas from DeviceSpec and DeviceBySpec. This is a prototype implementation, and further refactoring is expected. The key feature is clear composition of device specifications for a declarative description of devices and their behavior. Moreover, the API for parent and child devices (specs) is the same.
kolpakov.mm added 2 commits 2024-12-19 11:50:27 +03:00
Owner

Please fix MR name to better reflect what it does.

Please fix MR name to better reflect what it does.
altavir self-assigned this 2024-12-19 11:52:55 +03:00
altavir requested review from altavir 2024-12-19 11:53:00 +03:00
Owner

Also please add examples of new API usage both in code and in MR description.

Also please add examples of new API usage both in code and in MR description.
kolpakov.mm changed title from WIP: dev-maxim to WIP: Composition of specifications and devices 2024-12-19 11:57:55 +03:00
kolpakov.mm added 1 commit 2024-12-19 12:12:12 +03:00
Author
Member

Added examples of new API usage - CompositeControlTest based on Analyzer device with subdevices. In specification, define "by childSpec" for passing child device specification. In device, define "by childDevice" for getting child device. For now not properly working, not all tests are passed.

Added examples of new API usage - CompositeControlTest based on Analyzer device with subdevices. In specification, define "by childSpec" for passing child device specification. In device, define "by childDevice" for getting child device. For now not properly working, not all tests are passed.
altavir added the due date 2024-12-26 2024-12-19 12:59:06 +03:00
altavir requested changes 2024-12-25 12:21:14 +03:00
@ -0,0 +42,4 @@
/**
* Defines how child device lifecycle should be managed.
*/
public enum class LifecycleMode {
Owner

Seems like a bit of overengeneering to me. Could you provide examples, where all of theese are needed? Core API should be minimal

Seems like a bit of overengeneering to me. Could you provide examples, where all of theese are needed? Core API should be minimal
@ -0,0 +70,4 @@
public fun <D: ConfigurableCompositeControlComponent<*>> getSpec(name: Name): CompositeControlComponentSpec<D>?
}
public class ComponentRegistryManager : AbstractPlugin(), ComponentRegistry {
Owner

This seems to be a nice idea, but rather for remote configuration data base. What is the use case for this class locally? Should it be in the core?

This seems to be a nice idea, but rather for remote configuration data base. What is the use case for this class locally? Should it be in the core?
@ -0,0 +18,4 @@
/**
* Create a [MetaConverter] for enum values
*/
public fun <E : Enum<E>> createEnumConverter(enumValues: Array<E>): MetaConverter<E> = object : MetaConverter<E> {
Owner

Scope polution here. Please move to MetaConverter.Companion

Scope polution here. Please move to MetaConverter.Companion
@ -0,0 +18,4 @@
// ---------------------- Device Specifications ----------------------------------
public object StepperMotorSpec : CompositeControlComponentSpec<StepperMotorDevice>() {
public val position by intMutable(
Owner

The naming convention should be discussed

The naming convention should be discussed
@ -0,0 +110,4 @@
// ---------------------- Device Implementations ----------------------------------
// Implementation of Stepper Motor Device
public class StepperMotorDevice(
Owner

How is it different from regular Spec?

How is it different from regular Spec?
Owner

The main problem for me here is that it is not clear, how Composite devices are better than regular ones. I would like to mimimize the API border for the core.

@kolpakov.mm could you split this MR on two:

  • Descriptor builders - it could be merged immediately.
  • MR for composite devices, it could be discussed further. Maybee create a separate module for it?
The main problem for me here is that it is not clear, how Composite devices are better than regular ones. I would like to mimimize the API border for the core. @kolpakov.mm could you split this MR on two: * Descriptor builders - it could be merged immediately. * MR for composite devices, it could be discussed further. Maybee create a separate module for it?
kolpakov.mm added 1 commit 2024-12-25 18:11:47 +03:00
kolpakov.mm added 2 commits 2025-01-27 17:00:38 +03:00
- Implemented new RestartPolicy logic with exponential backoff support.
- Added systemBus as an extra stream for system-level events and logs.
- Improved DeviceStateEvent to handle more detailed device transitions.
kolpakov.mm added 1 commit 2025-01-28 01:44:56 +03:00
- Moved registration logic to `provideDelegate()`, ensuring child specs are
  registered upon declaration without needing explicit access.
kolpakov.mm added 1 commit 2025-01-28 22:02:19 +03:00
kolpakov.mm added 1 commit 2025-01-30 13:36:54 +03:00
kolpakov.mm added 3 commits 2025-02-01 05:21:58 +03:00
kolpakov.mm added 1 commit 2025-02-04 20:51:15 +03:00
kolpakov.mm added 1 commit 2025-02-06 12:55:59 +03:00
kolpakov.mm added 1 commit 2025-02-06 13:05:36 +03:00
kolpakov.mm added 1 commit 2025-02-07 09:26:59 +03:00
kolpakov.mm added 1 commit 2025-02-10 17:13:12 +03:00
kolpakov.mm added 1 commit 2025-02-11 16:24:54 +03:00
kolpakov.mm added 1 commit 2025-03-15 16:54:33 +03:00
- DeviceHubManager is now abstract
- StandardDeviceHubManager is the concrete implementation
- Logic for registry/health checks/error handling is separated into dedicated managers
- Introduced ControlsConfiguration for global settings and SystemResource for concurrency management
kolpakov.mm added 1 commit 2025-03-21 18:35:37 +03:00
- Moved CompositeControlComponents to dedicated module controls-composite-spec
- Improved separation of concerns between core device functionality and composite device architecture
altavir requested changes 2025-03-23 09:51:46 +03:00
altavir left a comment
Owner

There are lot of questions. The main issue I see right now is that significant part of distributed functionality that was intended for Magix is now re-implemented from scratch. Since the module is not not in the core, I recommend import magix and magix-utils and implement all this functionality on top of Magix.

The core feature is you can add new types of Magix message: just add a type, payload content and new MagixMessageFormat. It could elegantly solve all the problems with system events, log events etcetera.

There are lot of questions. The main issue I see right now is that significant part of distributed functionality that was intended for Magix is now re-implemented from scratch. Since the module is not not in the core, I recommend import magix and magix-utils and implement all this functionality on top of Magix. The core feature is you can add new types of Magix message: just add a type, payload content and new `MagixMessageFormat`. It could elegantly solve all the problems with system events, log events etcetera.
@ -0,0 +1,3287 @@
@file:Suppress("MemberVisibilityCanBePrivate", "unused")
Owner

This file is too large. Please split it into separate classes.

This file is too large. Please split it into separate classes.
@ -0,0 +24,4 @@
import kotlin.time.Duration.Companion.seconds
/** Global configuration for controls system. */
public class ControlsConfiguration(
Owner

Configurations like this should be passed as DeviceManager plugin parameters. Better to convert them into DeviceManager extensions. This is needed to make the whole system installable from configuration.

Configurations like this should be passed as DeviceManager plugin parameters. Better to convert them into DeviceManager extensions. This is needed to make the whole system installable from configuration.
Author
Member

Global ControlsConfiguration has been removed. Now the plugin is DeviceManagerConfig, which is registered in the context and is accessible via context.deviceManagerConfig.

Global ControlsConfiguration has been removed. Now the plugin is DeviceManagerConfig, which is registered in the context and is accessible via context.deviceManagerConfig.
@ -0,0 +56,4 @@
* @param T The type parameter of the deferred result.
* @return The completed value if available and not cancelled, or `null` otherwise.
*/
private suspend fun <T> Deferred<T>.awaitCompletedOrNull(): T? =
Owner

This is strange extension. It will return null if task is completed.

This is strange extension. It will return null if task is completed.
Author
Member

Deleted

Deleted
@ -0,0 +75,4 @@
/**
* Base class for all device-related exceptions
*/
Owner

I think exceptions should be moved to a separate file

I think exceptions should be moved to a separate file
@ -0,0 +76,4 @@
/**
* Base class for all device-related exceptions
*/
public sealed class DeviceException(
Owner

Shouldn't it be done via inheritance?

Shouldn't it be done via inheritance?
Author
Member

Regarding the DeviceException organization, I'm not sure yet how to do it better. See the last commit

Regarding the DeviceException organization, I'm not sure yet how to do it better. See the last commit
@ -0,0 +153,4 @@
/**
* EventBus interface for publishing and subscribing to application-level events.
*/
public interface EventBus {
Owner

Does it make sense to dublicate the whole bus infrastructure when we already have Magix bus? They are implemented on a different level: Magix operates on inter-device level, but that makes it more flexible. Also, it allows remote monitoring. At this point it does not make a lot of sense keeping two separate busses.

Does it make sense to dublicate the whole bus infrastructure when we already have Magix bus? They are implemented on a different level: Magix operates on inter-device level, but that makes it more flexible. Also, it allows remote monitoring. At this point it does not make a lot of sense keeping two separate busses.
Author
Member

EventBus and TransportAdapter removed. Made MessageBus with InMemoryMessageBus and MagixMessageBus implementations. DeviceHubManager uses MessageBus (by default MagixMessageBus if Magix is ​​available, otherwise InMemoryMessageBus). To make it easier to use without Magix in base

EventBus and TransportAdapter removed. Made MessageBus with InMemoryMessageBus and MagixMessageBus implementations. DeviceHubManager uses MessageBus (by default MagixMessageBus if Magix is ​​available, otherwise InMemoryMessageBus). To make it easier to use without Magix in base
@ -0,0 +176,4 @@
/**
* Event emitted when a message is sent through the transport
*/
public data class MessageSentEvent(val message: DeviceMessage) : Event
Owner

Could it be replaced with EmptyDeviceMessage?

Could it be replaced with `EmptyDeviceMessage`?
Author
Member

Actually I've removed it now

Actually I've removed it now
@ -0,0 +230,4 @@
* TransportAdapter interface for distributed communications.
* This abstraction allows plugging in different transport mechanisms.
*/
public interface TransportAdapter {
Owner

This is a direct replica of Magix bus. I think, Magix endpoint should be used instead.

This is a direct replica of Magix bus. I think, Magix endpoint should be used instead.
Author
Member

I removed it too

I removed it too
@ -0,0 +303,4 @@
/**
* Represents an undoable action for transaction rollback
*/
public interface UndoableAction {
Owner

Better to name it reversible. Or it reads as action that cannot be done.

Also reversing actions is a complex concept, it should be discussed separately.

Better to name it reversible. Or it reads as action that cannot be done. Also reversing actions is a complex concept, it should be discussed separately.
Author
Member

Interface renamed to ReversibleAction

Interface renamed to ReversibleAction
@ -0,0 +319,4 @@
* Represents a transaction context with a unique ID.
* This is used to track and manage transactions.
*/
public class TransactionContext(
Owner

Tranactions are part of storage. I think this should be a complete separate thing, maybe on storage side.

Tranactions are part of storage. I think this should be a complete separate thing, maybe on storage side.
@ -0,0 +350,4 @@
/**
* Interface for managing transactional operations.
*/
public interface TransactionManager {
Owner

At the very least, tranactions should be mover into a separate file. And in general it should be done in a separate MR.

At the very least, tranactions should be mover into a separate file. And in general it should be done in a separate MR.
@ -0,0 +488,4 @@
/**
* Interface for publishing metrics.
*/
public interface MetricPublisher {
Owner

It is better to do that in a separate type of Magix message

It is better to do that in a separate type of Magix message
Author
Member

MetricPublisherImpl now publishes MetricMessages (with subtypes Value, Counter, Duration, etc.) via MessagingSystem, which can be Magix

MetricPublisherImpl now publishes MetricMessages (with subtypes Value, Counter, Duration, etc.) via MessagingSystem, which can be Magix
@ -0,0 +596,4 @@
/**
* Thread-safe atomic reference implementation.
*/
public class AtomicReference<T>(initialValue: T) {
Owner

Please use https://github.com/Kotlin/kotlinx-atomicfu for that, do not reinvent the wheel.

Please use https://github.com/Kotlin/kotlinx-atomicfu for that, do not reinvent the wheel.
Author
Member

Fixed, now atomic is from the library.

Fixed, now atomic is from the library.
@ -0,0 +658,4 @@
/**
* Interface for loading external device configurations.
*/
public interface ExternalConfigurationProvider {
Owner

I think it could be done with MagixRegistry from magix-utils module.

I think it could be done with `MagixRegistry` from `magix-utils` module.
Author
Member

Interface removed. Configuration is now passed via Meta

Interface removed. Configuration is now passed via Meta
@ -0,0 +1032,4 @@
*
* @param CD The type of the child device.
*/
public interface ChildComponentConfig<CD : ConfigurableCompositeControlComponent<CD>> {
Owner

We need to try to follow DataForge ideology here. Namely that everything could be created using context and metadata. Using any stateful objects for configuration could break automatic object creation.

We need to try to follow DataForge ideology here. Namely that everything could be created using context and metadata. Using any stateful objects for configuration could break automatic object creation.
Author
Member

Added fromMeta method to create config from Meta, if that's appropriate.

Added fromMeta method to create config from Meta, if that's appropriate.
@ -0,0 +1048,4 @@
*
* @param D The device type this spec applies to.
*/
public interface CompositeDeviceSpec<D : ConfigurableCompositeControlComponent<D>> {
Owner

Could it inherit regular DeviceSpec?

Could it inherit regular DeviceSpec?
Author
Member

There's a class there, and here's an interface, so I didn't touch it for now

There's a class there, and here's an interface, so I didn't touch it for now
@ -0,0 +1543,4 @@
/**
* Manager for health checks across devices.
*/
public class HealthCheckManager(
Owner

This should be movedd to a separate file. I also propose to use Magix for that. Since now you are not in the core, you can import Magix here.

This should be movedd to a separate file. I also propose to use Magix for that. Since now you are not in the core, you can import Magix here.
@ -0,0 +1647,4 @@
/**
* Manages error handling for devices.
*/
public class ErrorHandlingManager(
Owner

This also should be separated and probably moved on top of magix. It is hard to implement distributed device registry, but I wonder if it is needed.

This also should be separated and probably moved on top of magix. It is hard to implement distributed device registry, but I wonder if it is needed.
@ -0,0 +1978,4 @@
/**
* A [MutableSharedFlow] for system-level messages.
*/
public abstract val systemBus: MutableSharedFlow<SystemLogMessage>
Owner

Why do you need separate busses for different messages? Just create single bus with different types of magix messages.

Why do you need separate busses for different messages? Just create single bus with different types of magix messages.
Author
Member

Replaced with MessagingSystem / MessageBus. Different message types can be accessed via the required flows (getDeviceLogMessages, getSystemLogMessages)

Replaced with MessagingSystem / MessageBus. Different message types can be accessed via the required flows (getDeviceLogMessages, getSystemLogMessages)
@ -0,0 +10,4 @@
/**
* Create a [MetaConverter] for enum values using [reified] type with an option to ignore case.
*/
public inline fun <reified E : Enum<E>> createEnumConverter(ignoreCase: Boolean = false): MetaConverter<E> {
Owner

There is already MetaConverter.enum for that. Please explain why it is not applicable.

There is already MetaConverter.enum for that. Please explain why it is not applicable.
Author
Member

Here I am being a bit clever, perhaps, it's just that in the original in readOrNull it's somehow strange, an error may pop up, and not "null" in return. For me it returns null, and it works taking into account ignoreCase. But I'm not sure that this is very important or rather, yes. I just investigated.

Here I am being a bit clever, perhaps, it's just that in the original in readOrNull it's somehow strange, an error may pop up, and not "null" in return. For me it returns null, and it works taking into account ignoreCase. But I'm not sure that this is very important or rather, yes. I just investigated.
Owner

Should be moved to MetaConverter.Companion to avoid global scope pollution.

Should be moved to MetaConverter.Companion to avoid global scope pollution.
@ -219,1 +219,4 @@
@Serializable
@SerialName("system.log")
public data class SystemLogMessage(
Owner

How is it different from DeviceLogMessage?

How is it different from DeviceLogMessage?
Author
Member

According to the old idea, SystemLogMessage is for system-level logs, and DeviceLogMessage is for logs of a specific device (sourceDevice) and can carry Meta. Not sure what is the best idea, maybe combine it with the "level" field (device/system).

According to the old idea, SystemLogMessage is for system-level logs, and DeviceLogMessage is for logs of a specific device (sourceDevice) and can carry Meta. Not sure what is the best idea, maybe combine it with the "level" field (device/system).
@ -11,0 +11,4 @@
/**
* The device is newly created and has not started yet.
*/
INITIAL,
Owner

Maybe INIT?

Maybe INIT?
@ -6,3 +6,3 @@
import space.kscience.dataforge.meta.MetaConverter
internal object DeviceMetaPropertySpec : DevicePropertySpec<Device, Meta> {
public object DeviceMetaPropertySpec : DevicePropertySpec<Device, Meta> {
Owner

Is it necessary?

Is it necessary?
Author
Member

No)

No)
Owner

Then put it back.

Then put it back.
@ -94,3 +94,4 @@ include(
":demo:constructor",
":demo:device-collective"
)
include("controls-composite-spec")
Owner

Add this to the list above. Before demos

Add this to the list above. Before demos
Author
Member

Fixed

Fixed
altavir changed title from WIP: Composition of specifications and devices to Composition of specifications and devices 2025-03-23 09:54:33 +03:00
altavir pinned this 2025-03-23 11:36:55 +03:00
altavir removed the due date 2024-12-26 2025-03-23 11:37:02 +03:00
Member

Привет. Я посмотрел изменения, и у меня есть несколько вопросов:

  1. Я не увидел в примерах аргументов при создании дочернего устройства. Мб добавить пример с аргументами?
  2. Не очень понятна область видимости. Можно ли вызывать подустройства из другого устройства или Magix? Предполагается ли такая возможность?
  3. Может вместо тестов или в дополнение к ним сделать пример в demo с инструкцией как запускать и что должно отображаться? Если сделать демку с графическим интерфейсом то 2. отпадет автоматически.
Привет. Я посмотрел изменения, и у меня есть несколько вопросов: 1. Я не увидел в примерах аргументов при создании дочернего устройства. Мб добавить пример с аргументами? 2. Не очень понятна область видимости. Можно ли вызывать подустройства из другого устройства или Magix? Предполагается ли такая возможность? 3. Может вместо тестов или в дополнение к ним сделать пример в demo с инструкцией как запускать и что должно отображаться? Если сделать демку с графическим интерфейсом то 2. отпадет автоматически.
kolpakov.mm added 5 commits 2025-03-29 21:54:46 +03:00
kolpakov.mm added 2 commits 2025-04-14 23:59:40 +03:00
Author
Member

Привет. Я посмотрел изменения, и у меня есть несколько вопросов:

  1. Я не увидел в примерах аргументов при создании дочернего устройства. Мб добавить пример с аргументами?
  2. Не очень понятна область видимости. Можно ли вызывать подустройства из другого устройства или Magix? Предполагается ли такая возможность?
  3. Может вместо тестов или в дополнение к ним сделать пример в demo с инструкцией как запускать и что должно отображаться? Если сделать демку с графическим интерфейсом то 2. отпадет автоматически.
  1. ChildComponentConfig содержит поле meta, которое передается при создании/аттаче дочернего устройства в ConfigurableCompositeControlComponent.initChildren. Это по задумке позволит передавать конфигурацию.
  2. Подустройства доступны через родительский ConfigurableCompositeControlComponent (поле devices) или напрямую через DeviceHubManager (если есть доступ к нему). Через Magix тоже, если родительское устройство или DeviceHubManager подключены к Magix. Имя дочернего устройства будет parentName.childName.
  3. Демка обязательно будет, сделаю, сейчас больше надо всё дорефакторить в некий согласованный вид
> Привет. Я посмотрел изменения, и у меня есть несколько вопросов: > > 1. Я не увидел в примерах аргументов при создании дочернего устройства. Мб добавить пример с аргументами? > 2. Не очень понятна область видимости. Можно ли вызывать подустройства из другого устройства или Magix? Предполагается ли такая возможность? > 3. Может вместо тестов или в дополнение к ним сделать пример в demo с инструкцией как запускать и что должно отображаться? Если сделать демку с графическим интерфейсом то 2. отпадет автоматически. 1. ChildComponentConfig содержит поле meta, которое передается при создании/аттаче дочернего устройства в ConfigurableCompositeControlComponent.initChildren. Это по задумке позволит передавать конфигурацию. 2. Подустройства доступны через родительский ConfigurableCompositeControlComponent (поле devices) или напрямую через DeviceHubManager (если есть доступ к нему). Через Magix тоже, если родительское устройство или DeviceHubManager подключены к Magix. Имя дочернего устройства будет parentName.childName. 3. Демка обязательно будет, сделаю, сейчас больше надо всё дорефакторить в некий согласованный вид
altavir added the due date 2025-04-21 2025-04-17 11:04:26 +03:00
altavir requested changes 2025-04-20 09:13:26 +03:00
altavir left a comment
Owner

The changes in the Devica Message hierarchy are not approved. Otherwise looks fine.

The changes in the Devica Message hierarchy are not approved. Otherwise looks fine.
@ -0,0 +22,4 @@
implementation(projects.magix.magixRsocket)
implementation(projects.magix.magixZmq)
implementation(projects.controlsMagix)
implementation("org.jetbrains.kotlinx:atomicfu:0.27.0")
Owner

Is it necessary?

Is it necessary?
@ -19,3 +20,3 @@
@Serializable
public sealed class DeviceMessage {
public sealed class Message {
Owner

Why do you need this? To avoid writing comment? Just leave it blank. I do not think this change is necessary.

Why do you need this? To avoid writing comment? Just leave it blank. I do not think this change is necessary.
This pull request has changes conflicting with the target branch.
  • controls-core/src/commonMain/kotlin/space/kscience/controls/api/DeviceMessage.kt
  • controls-core/src/commonMain/kotlin/space/kscience/controls/spec/DeviceBase.kt
  • controls-core/src/wasmJsMain/kotlin/fromSpec.wasm.kt
  • demo/motors/src/main/kotlin/ru/mipt/npm/devices/pimotionmaster/PiMotionMasterDevice.kt

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin dev-maxim:dev-maxim
git checkout dev-maxim
Sign in to join this conversation.
No description provided.