Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

Fix Mi Flora #40

Merged
merged 5 commits into from Oct 21, 2017
Merged

Fix Mi Flora #40

merged 5 commits into from Oct 21, 2017

Conversation

sharukins
Copy link
Contributor

Extract missing gatttool which is needed by the Mi Flora component from the edge package.. This is a temporary fix until Alpine 3.7 is released.

Extract missing gatttool which is needed by the Mi Flora component from the edge package.. This is a temporary fix until Alpine 3.7 is released.
@homeassistant
Copy link

Hi @sharukins,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

Copy link
Member

@pvizeli pvizeli left a comment

Choose a reason for hiding this comment

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

Is that testet?

## Install gatttool from edge (can be removed with release of Alpine 3.7)
RUN mkdir -p bluez-deprecated \
&& cd bluez-deprecated \
&& curl -o bluez-deprecated-5.47-r3.apk http://dl-3.alpinelinux.org/alpine/edge/main/armhf/bluez-deprecated-5.47-r3.apk \
Copy link
Member

Choose a reason for hiding this comment

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

You need write that for all arch. You can use $BUILD_ARCH they include (amd64, aarch64, armhf, i386)

@sharukins
Copy link
Contributor Author

I didn't test it with home assistant, but bluez-deprecated does definitely contain gatttool and I did unpack it using tar from the apk. Therefore I'm pretty sure this works.

Not quite elegant, but should do the job.
rm -fr bluez-deprecated; \
else cd ../; \
rm- fr bluez-deprecated; \
exit 0; \
Copy link
Member

Choose a reason for hiding this comment

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

I think we can handle that cleaner like:

&& if ...; then APK_ARCH=...; \
&& curl -so ... http://.../$APK_ARCH/....

This sould be more clean than the version before. 
I check $APK_ARCH again just in case. not sure if this is really necessary.
Copy link
Member

@pvizeli pvizeli left a comment

Choose a reason for hiding this comment

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

Good work 👍 we can do it very simple and only add the correct arch detection with you first code

elif [ "$BUILD_ARCH" = "armhf" ]; then APK_ARCH = "armhf"; \
elif [ "$BUILD_ARCH" = "aarch64" ]; then APK_ARCH = "aarch64"; \
else exit 0; \
&& if [ -z "$APK_ARCH"]; then cd ../; \
Copy link
Member

Choose a reason for hiding this comment

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

That will be set, this is not needed

@pvizeli
Copy link
Member

pvizeli commented Oct 21, 2017

If you finish it soon we can add it into 0.56

@pvizeli pvizeli merged commit 21bc6d3 into home-assistant:master Oct 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants