Skip to content

Bugfix/#43 fix controller restart on config change#114

Merged
DivineThreepwood merged 18 commits intodevfrom
bugfix/#43_fix_controller_restart_on_config_change
Aug 3, 2023
Merged

Bugfix/#43 fix controller restart on config change#114
DivineThreepwood merged 18 commits intodevfrom
bugfix/#43_fix_controller_restart_on_config_change

Conversation

@DivineThreepwood
Copy link
Member

@DivineThreepwood DivineThreepwood commented Jul 26, 2023 •
edited
Loading

📜 Description

❗ Blocked by: openbase/jul#104

Code changes proposed during fixing issue #43 that is actually fixed in Jul PR openbase/jul#104

  • fix some linting issues
  • fix some generic class declarations.
  • improve registry shutdown.
  • Port UnitRemotePool to Koltin
  • Fix update issue of unit remote pool.

github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 26, 2023
View reviewed changes
module/dal/remote/src/main/java/org/openbase/bco/dal/remote/layer/unit/CustomUnitPool.kt
* #L%
*/

class CustomUnitPool : Manageable<Collection<Filter<UnitConfigType.UnitConfig>>> {

Check warning

Code scanning / detekt

Too many functions inside a/an file/class/object/interface always indicate a violation of the single responsibility principle. Maybe the file/class/object/interface wants to manage too many things at once. Extract functionality which clearly belongs together.

Class 'CustomUnitPool' with '14' functions detected. Defined threshold inside classes is set to '11'
module/dal/remote/src/main/java/org/openbase/bco/dal/remote/layer/unit/CustomUnitPool.kt
*/

class CustomUnitPool : Manageable<Collection<Filter<UnitConfigType.UnitConfig>>> {
private val UNIT_REMOTE_REGISTRY_LOCK = ReentrantReadWriteLock()

Check warning

Code scanning / detekt

Variable names should follow the naming convention set in the projects configuration.

Private variable names should match the pattern: (_)?[a-z][A-Za-z0-9]*
...e/api/graphql/src/main/java/org/openbase/bco/api/graphql/subscriptions/SubscriptionModule.kt

@Throws(BCOGraphQLError::class)
fun subscribeUnits(unitFilter: UnitFilter?): Publisher<UnitDataType.UnitData> {
fun subscribeUnits(unitFilter: UnitFilter): Publisher<UnitDataType.UnitData> {

Check warning

Code scanning / detekt

Restrict the number of throw statements in methods.

Too many throw statements in the function subscribeUnits. The maximum number of allowed throw statements is 2.
module/dal/remote/src/main/java/org/openbase/bco/dal/remote/layer/unit/CustomUnitPool.kt
import org.openbase.bco.dal.lib.layer.unit.UnitRemote
import org.openbase.bco.registry.remote.Registries
import org.openbase.bco.registry.unit.lib.filter.UnitConfigFilterImpl
import org.openbase.jul.exception.*

Check warning

Code scanning / detekt

Wildcard imports should be replaced with imports using fully qualified class names. Wildcard imports can lead to naming conflicts. A library update can introduce naming clashes with your classes which results in compilation errors.

org.openbase.jul.exception.* is a wildcard import. Replace it with fully qualified imports.
module/dal/remote/src/main/java/org/openbase/bco/dal/remote/layer/unit/CustomUnitPool.kt

class CustomUnitPool : Manageable<Collection<Filter<UnitConfigType.UnitConfig>>> {
private val UNIT_REMOTE_REGISTRY_LOCK = ReentrantReadWriteLock()
private var unitRegistryDataObserver: Observer<DataProvider<UnitRegistryDataType.UnitRegistryData>, UnitRegistryDataType.UnitRegistryData>

Check warning

Code scanning / detekt

Line detected that is longer than the defined maximum line length in the code style.

Line detected that is longer than the defined maximum line length in the code style.
module/dal/remote/src/main/java/org/openbase/bco/dal/remote/layer/unit/CustomUnitPool.kt
unitConfigDiff = ProtobufListDiff()
unitRemoteRegistry = RemoteControllerRegistry()
unitRegistryDataObserver =
Observer { source: DataProvider<UnitRegistryDataType.UnitRegistryData>, data: UnitRegistryDataType.UnitRegistryData? -> sync() }

Check warning

Code scanning / detekt

Line detected that is longer than the defined maximum line length in the code style.

Line detected that is longer than the defined maximum line length in the code style.
module/dal/remote/src/main/java/org/openbase/bco/dal/remote/layer/unit/CustomUnitPool.kt
* If the pool is already active, then the filter is directly applied.
* If you call this method twice, only the latest filter set is used.
*
* @param unitFilter this is a unit filter that can be used to limit the number of unit to observer. No filter means every unit is observed by this pool.

Check warning

Code scanning / detekt

Line detected that is longer than the defined maximum line length in the code style.

Line detected that is longer than the defined maximum line length in the code style.
module/dal/remote/src/main/java/org/openbase/bco/dal/remote/layer/unit/CustomUnitPool.kt
* If the pool is already active, then the filter is directly applied.
* If you call this method twice, only the latest filter set is used.
*
* @param filters this set of filters can be used to limit the number of unit to observer. No filter means every unit is observed by this pool.

Check warning

Code scanning / detekt

Line detected that is longer than the defined maximum line length in the code style.

Line detected that is longer than the defined maximum line length in the code style.
module/dal/remote/src/main/java/org/openbase/bco/dal/remote/layer/unit/CustomUnitPool.kt
* If the pool is already active, then the filter is directly applied.
* If you call this method twice, only the latest filter set is used.
*
* @param filters this set of filters can be used to limit the number of unit to observer. No filter means every unit is observed by this pool.

Check warning

Code scanning / detekt

Line detected that is longer than the defined maximum line length in the code style.

Line detected that is longer than the defined maximum line length in the code style.
module/dal/remote/src/main/java/org/openbase/bco/dal/remote/layer/unit/CustomUnitPool.kt
Registries.getUnitRegistry().removeDataObserver(unitRegistryDataObserver)
} catch (ex: NotAvailableException) {
// if the registry is not available an observer deregistration is not required.
// This can for example be the case when the unit registry has already been terminated during the shutdown progress.

Check warning

Code scanning / detekt

Line detected that is longer than the defined maximum line length in the code style.

Line detected that is longer than the defined maximum line length in the code style.
DivineThreepwood
DivineThreepwood commented Jul 26, 2023
View reviewed changes
Copy link
Member Author

@DivineThreepwood DivineThreepwood left a comment

Choose a reason for hiding this comment

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

from my side everything is fine.

@lhuxohl lhuxohl assigned DivineThreepwood and unassigned lhuxohl Jul 31, 2023
@DivineThreepwood DivineThreepwood requested a review from lhuxohl July 31, 2023 18:40
@DivineThreepwood DivineThreepwood added ready to review blocked PR is based on another PR and is therefore blocked until the other one has been merged. and removed changes requested labels Jul 31, 2023
lhuxohl
lhuxohl previously approved these changes Aug 1, 2023
View reviewed changes
lhuxohl
lhuxohl previously approved these changes Aug 3, 2023
View reviewed changes
@DivineThreepwood DivineThreepwood removed the blocked PR is based on another PR and is therefore blocked until the other one has been merged. label Aug 3, 2023
lhuxohl
lhuxohl previously approved these changes Aug 3, 2023
View reviewed changes
@DivineThreepwood DivineThreepwood merged commit d2a7cff into dev Aug 3, 2023
@DivineThreepwood DivineThreepwood deleted the bugfix/#43_fix_controller_restart_on_config_change branch August 3, 2023 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lhuxohl lhuxohl lhuxohl left review comments

@pLeminoq pLeminoq Awaiting requested review from pLeminoq

Assignees

@DivineThreepwood DivineThreepwood

Labels

code approved tested

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@DivineThreepwood @lhuxohl