Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bulk out Module support for specific requirements #1118

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ikelos
Copy link
Member

@ikelos ikelos commented Mar 24, 2024

This should ensure that modules more explicitly state what requirements they have which should also allow that to be recorded more accurately in the configuration when needed.

@Abyss-W4tcher please could you test this (after adding your own requirements to your new Module type) and report back whether the values you need get stored appropriately or not.

@Abyss-W4tcher
Copy link
Contributor

I will try it soon and keep you informed 👍

@Abyss-W4tcher
Copy link
Contributor

I have merged these changes, and tried on a basic sample mac-sample-1.bin, and adding random non-optional requirements isn't preventing the module from instantiating.

In fact, get_requirements() doesn't seem to be called in the first place.

@Abyss-W4tcher
Copy link
Contributor

Hi, any update on this feature ?

@ikelos
Copy link
Member Author

ikelos commented Apr 23, 2024

No, not yet sorry, I need to find time to unpick why get_requirements isn't getting called appropriately...

@ikelos
Copy link
Member Author

ikelos commented Apr 23, 2024

Ok, so my mistake. It turns out it's not the Module that's used to figure out its requirements, it's the ModuleRequirement which specifies them, meaning for a specific module to be loaded, we can't just create a new ArmModule with different requirements, we'd need to define a custom ArmModuleRequirement which somewhat does away with the point of trying to keep things modular. As such this branch is duff and I'm going to close it off.

In the larger picture of getting the Mac stuff in the right way, I'm afraid it's back to the drawing board for a little bit to figure out how to get optional requirements for a ModuleRequiement (I suspect it'll come down to a MacModuleRequirement, which isn't ideal, but may be workable)...

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.

2 participants