Skip to content

Conversation

@Pietfried
Copy link
Contributor

@Pietfried Pietfried commented Nov 27, 2025

Describe your changes

Added support for external slac API module:

  • Added slac_API module - External API for SLAC
  • Added BUSlac module - BringUp module for SLAC
  • Added SLAC API documentation

Issue ticket number and link

Original PR: #1463

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation
  • I read the contribution documentation and made sure that my changes meet its requirements

* Added slac_API module - External API for SLAC
* Added BUSlac module - BringUp module for SLAC
* Added API documentation

Signed-off-by: Piet Gömpel <pietgoempel@gmail.com>
Signed-off-by: Piet Gömpel <pietgoempel@gmail.com>
@Pietfried
Copy link
Contributor Author

@djchhp all changes of the original PR had been adressed.

Copy link
Contributor

@djchhp djchhp left a comment

Choose a reason for hiding this comment

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

looks good to me

});

screen.Loop(main_renderer);
}
Copy link
Member

@james-ctc james-ctc Dec 4, 2025

Choose a reason for hiding this comment

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

Is it documented somewhere that ready() doesn't need to return?
Other than in the bringup modules I've not seen this pattern. My concern is that it is making an assumption about how the framework works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Afaik it's not documented. The documentation of the framework including startup behavior, module communication, cmd/var handling, etc. should in general be improved. I created an issue for this: #1613

@Pietfried Pietfried disabled auto-merge December 4, 2025 18:01
@Pietfried Pietfried enabled auto-merge December 4, 2025 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants