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

sj201 v6 and v10 division #32

Merged
merged 3 commits into from
Jun 2, 2024
Merged

Conversation

builderjer
Copy link
Member

added sj201v10 detection.

This helps with plugin validation.

added sj201v10 detection.

This helps with plugin validation.
@builderjer
Copy link
Member Author

builderjer commented Jun 1, 2024

Is this failing because it is from my personal repo? and not from ovos?

Regardless, I think this should be merged until we figure out where this code can go.

@JarbasAl
Copy link
Member

JarbasAl commented Jun 1, 2024

i still think these should be defined in the individual plugins and this file deleted from here

only the mk2 plugins need to check for a mk2.... doesnt make sense to be in PHAL.

also plugins should only need to import from OPM, not from the PHAL code (this is true for all plugins and other services)

@builderjer
Copy link
Member Author

I feel that it should be shared code somewhere in the ovos repos. OPM makes sense to me more than individual packages copying the same thing. This allows for multiple mk2 plug-ins to do an import, instead of duplicating code.

@JarbasAl
Copy link
Member

JarbasAl commented Jun 1, 2024

1 line of code 'i2cdetect -y -a 1 0x04 0x04 | egrep "(04|UU)" | awk \'{print $2}\'' used in 2 repos at best (fan and leds) is really not worth packaging IMHO....

conceptually doesnt fit in OPM repos either, in the sense it is not a shared util for all plugins, same argument done for the led and fan stuff

perhaps a dedicated tiny library can be created, just to have the i2c checks available in a single place with 0 dependencies, making it useful also for non OVOS projects etc

@builderjer
Copy link
Member Author

I can go for that for sure

@builderjer
Copy link
Member Author

depreciated detection to move to its own repo
https://github.com/OpenVoiceOS/ovos-i2c-detection

@JarbasAl JarbasAl closed this Jun 2, 2024
@JarbasAl JarbasAl reopened this Jun 2, 2024
@JarbasAl JarbasAl merged commit 18a091d into OpenVoiceOS:dev Jun 2, 2024
11 of 13 checks passed
@JarbasAl JarbasAl mentioned this pull request Jun 2, 2024
@builderjer builderjer deleted the feat/sj201_v6_v10 branch June 2, 2024 13:29
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