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

fix: 3816 - use a 3.2.0 mlkit version of mobile_scanner #3833

Merged
merged 2 commits into from
Mar 30, 2023

Conversation

monsieurtanuki
Copy link
Contributor

What

  • Implementing @juliansteenbakker's suggestion about mlkit pod.
  • Of course one day we'll have to fix it in a more decent manner, but let's try this one!

Part of

@monsieurtanuki monsieurtanuki merged commit bc5feed into openfoodfacts:develop Mar 30, 2023
6 checks passed
@monsieurtanuki
Copy link
Contributor Author

Fingers crossed...

@monsieurtanuki
Copy link
Contributor Author

It worked, didn't it?

@juliansteenbakker
Copy link

Looks like it worked! Ill keep the branch thats being used right now in sync with the master branch of mobile_scanner, but i'd still recommend dropping the conflicting camerawesome package so you can use the pub.dev version.

@monsieurtanuki
Copy link
Contributor Author

camerawesome is so far only an option we're considering, as we are currently in a "relevant barcode scanners" test/benchmark phase.
It becomes less attractive if there are conflicts with the already used packages. And those conflicts are not easy to track as @M123-dev develops around camerawesome and doesn't use a mac.

@M123-dev
Copy link
Member

M123-dev commented Apr 1, 2023

Yes my guess with camera awesome was: since it's a middle thing between the bare bones flutter:camera and a fully integrated scanner it would make it simpler to use but at the same time more flexible to do things as moving the focus point (see my issue in the repo, it is possible with a bit of hacking around)

@M123-dev
Copy link
Member

M123-dev commented Apr 1, 2023

While talking about this, while we have loose discussions about if we should change the main use of the scanner the first thing we talked about is the scanner itself, but does not matter what it will get, a "full-screen" scanner would definitely look better than cutting it halfway through.

Is there a way to move the focus point up a bit in the upper two thirds of the screen in mobile_scanner @juliansteenbakker

@monsieurtanuki
Copy link
Contributor Author

a "full-screen" scanner would definitely look better than cutting it halfway through.

That's really a matter of appreciation. As an engineer the extra work needed for making the camera work full screen with a focus on top, only to have 90% of the bottom camera output hidden by product cards, does not seem relevant.
Let alone the performance issues (more memory, more code, more processing needed) and the history of bugs (especially given the android diversity).
To me the problem is not with mlkit, zxing or camerawesome, the problem is with the carousel, cf. #3777. As a Smoothie user I never actually used the carousel (I use history instead), and I cannot name an app with a barcode/qrcode scanner where the camera/scan is not the biggest and central widget of the screen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants