AndriousSolutions / ads

Other
58 stars 11 forks source link

New versions display only test ads in release mode... #17

Closed SirJohn2024 closed 4 years ago

SirJohn2024 commented 4 years ago

I recently update the plugin from version 1.0.1 to 1.2 and I have this strange behavior: every ad displayed on device, with the release mode app, is actually a test ad. Same for version 1.1.0, seems only the 1.0.1 maybe check the build mode or the Ads testing: parameter... I don't know.. I tested on 3 version of Android and had the same results..

Andrious commented 4 years ago

I'll take a look.

Andrious commented 4 years ago

I'm not getting the same issues as you. In a effort to help you, I must ask a few questions. I don't want to look at your app, but I must ask if your app is published yet on the App store and alike. Is it a published app?

Again, you're stating that version 1.0.1 is displaying 'production' ads while more recent versions are displaying test ads. Is that right?

Does the 'test ads' in question declare themselves as test ads. Otherwise, do the ads appear to be advertising a service and or item as an ad should and yet have a small label in the centre top displaying, 'test ad.' There is a subtle difference.

Would you supply some screenshots? You're allowed to drag and drop such files here.

SirJohn2024 commented 4 years ago

Thank you for your answer…

It’s clear to me that in debug/test the ad is displayed with the “Test Ad” tag on top.

My app is actually on the Google App Store, I replaced an old java version with the new Flutter one.

I made some test on these devices:

The sample app apk is built with a release key, the only thing changing the ads version (see attached files). The build process is Packages get/upgrade, flutter clean, debug build, then on the Android side: Clean Project, Generate Signed Bundle/APK and test on devices.

Here the result for version 1.0.1:

And for the 1.1.0/1.2.0:

As you can see, the 1.0.1 displays the correct ad on Android 10 but no one on the older devices (that can be a bug in firebase_admob plugin, don’t know), while the ads 1.1.0/1.2.0 plugins display a test ad.

Maybe it’s something that depends on my AdMob profile or the way I use the plugin, but something is obviously wrong…

Thanks for your work…

Best regards,

Sergio

From: Andrious Solutions <notifications@github.com mailto:notifications@github.com > Sent: mercoledì 16 ottobre 2019 03:15 To: AndriousSolutions/ads <ads@noreply.github.com mailto:ads@noreply.github.com > Cc: Sergio <sergio.bassan.development@gmail.com mailto:sergio.bassan.development@gmail.com >; Author <author@noreply.github.com mailto:author@noreply.github.com > Subject: Re: [AndriousSolutions/ads] New versions display only test ads in release mode... (#17)

I'm not getting the same issues as you. In a effort to help you, I must ask a few questions. I don't want to look at your app, but I must ask if your app is published yet on the App store and alike. Is it a published app?

Again, you're stating that version 1.0.1 is displaying 'production' ads while more recent versions are displaying test ads. Is that right?

Does the 'test ads' in question declare themselves as test ads. Otherwise, do the ads appear to be advertising a service and or item as an ad should and yet have a small label in the centre top displaying, 'test ad.' There is a subtle difference.

Would you supply some screenshots? You're allowed to drag and drop such files here.

— You are receiving this because you authored the thread. Reply to this email directly, https://github.com/AndriousSolutions/ads/issues/17?email_source=notifications&email_token=AABTS2JDG2TLQPXU2LVXRETQOZTHXA5CNFSM4I7ZD2R2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBKWO5I#issuecomment-542467957 view it on GitHub, or https://github.com/notifications/unsubscribe-auth/AABTS2MPDLVUHKTQQDHRYJLQOZTHXANCNFSM4I7ZD2RQ unsubscribe. https://github.com/notifications/beacon/AABTS2IGDTEFUJ2D3X5NNWTQOZTHXA5CNFSM4I7ZD2R2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBKWO5I.gif

SirJohn2024 commented 4 years ago

I forgot the attachments… :-/

From: sergio.bassan.development@gmail.com sergio.bassan.development@gmail.com Sent: mercoledì 16 ottobre 2019 15:40 To: 'AndriousSolutions/ads' reply@reply.github.com Cc: Sergio Bassan sergio.bassan@gmail.com Subject: RE: [AndriousSolutions/ads] New versions display only test ads in release mode... (#17)

Thank you for your answer…

It’s clear to me that in debug/test the ad is displayed with the “Test Ad” tag on top.

My app is actually on the Google App Store, I replaced an old java version with the new Flutter one.

I made some test on these devices:

The sample app apk is built with a release key, the only thing changing the ads version (see attached files). The build process is Packages get/upgrade, flutter clean, debug build, then on the Android side: Clean Project, Generate Signed Bundle/APK and test on devices.

Here the result for version 1.0.1:

And for the 1.1.0/1.2.0:

As you can see, the 1.0.1 displays the correct ad on Android 10 but no one on the older devices (that can be a bug in firebase_admob plugin, don’t know), while the ads 1.1.0/1.2.0 plugins display a test ad.

Maybe it’s something that depends on my AdMob profile or the way I use the plugin, but something is obviously wrong…

Thanks for your work…

Best regards,

Sergio

From: Andrious Solutions <notifications@github.com mailto:notifications@github.com > Sent: mercoledì 16 ottobre 2019 03:15 To: AndriousSolutions/ads <ads@noreply.github.com mailto:ads@noreply.github.com > Cc: Sergio <sergio.bassan.development@gmail.com mailto:sergio.bassan.development@gmail.com >; Author <author@noreply.github.com mailto:author@noreply.github.com > Subject: Re: [AndriousSolutions/ads] New versions display only test ads in release mode... (#17)

I'm not getting the same issues as you. In a effort to help you, I must ask a few questions. I don't want to look at your app, but I must ask if your app is published yet on the App store and alike. Is it a published app?

Again, you're stating that version 1.0.1 is displaying 'production' ads while more recent versions are displaying test ads. Is that right?

Does the 'test ads' in question declare themselves as test ads. Otherwise, do the ads appear to be advertising a service and or item as an ad should and yet have a small label in the centre top displaying, 'test ad.' There is a subtle difference.

Would you supply some screenshots? You're allowed to drag and drop such files here.

— You are receiving this because you authored the thread. Reply to this email directly, https://github.com/AndriousSolutions/ads/issues/17?email_source=notifications&email_token=AABTS2JDG2TLQPXU2LVXRETQOZTHXA5CNFSM4I7ZD2R2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBKWO5I#issuecomment-542467957 view it on GitHub, or https://github.com/notifications/unsubscribe-auth/AABTS2MPDLVUHKTQQDHRYJLQOZTHXANCNFSM4I7ZD2RQ unsubscribe. https://github.com/notifications/beacon/AABTS2IGDTEFUJ2D3X5NNWTQOZTHXA5CNFSM4I7ZD2R2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBKWO5I.gif

SirJohn2024 commented 4 years ago

Opsss... no images attached... Give me an email if want to take a look at them...

SirJohn2024 commented 4 years ago

I tested the sample app also on iOS and the results are the same: 1.0.1 Ok in release mode, test ads with the new ones...

The plugin always displays the following error:

flutter: Debug build: false
<Google> Invalid application ID. Follow instructions at https://goo.gl/QJ4sUK to find your app ID in the AdMob UI.
FirebaseAdMobPlugin <warning> Size Type: AdSizeType.WidthAndHeight

but, despite the "Invalid App id" error, the 1.0.1 works ok...

Can send the screenshots if you're interested...

Andrious commented 4 years ago

Yes, please send all the screenshots you suggested to the following email address: support 'at' andrioussolutions.com

thank-you.

Andrious commented 4 years ago

I tested the sample app also on iOS and the results are the same: 1.0.1 Ok in release mode, test ads with the new ones...

The plugin always displays the following error:

flutter: Debug build: false
<Google> Invalid application ID. Follow instructions at https://goo.gl/QJ4sUK to find your app ID in the AdMob UI.
FirebaseAdMobPlugin <warning> Size Type: AdSizeType.WidthAndHeight

but, despite the "Invalid App id" error, the 1.0.1 works ok...

Can send the screenshots if you're interested...

I'll soon begin testing ads in production once again. Until then, with regards with this 'Invalid application ID', would your AndroidManifest file need to be updated to resolve this particular issue?

Google AdMob (Android) Update your AndroidManifest.xml

Google AdMob All Apps

Andrious commented 4 years ago

In fact, looking further into the "invalid App id" issue, that would be the reason why you're getting 'test ads' in production. There's no App id supplied to the constructor. Further, in these most recent versions of Ads, you can instantiate only one instance of the Ads object. However, there's no Assert statement to warn you of this. The app will merely not display any ads. The screenshot of the constructor below shows you how a test app id is instead used if no other is supplied: appidNull Possibly this is what you are witnessing in your app. It's explained further in my article, My Code’s Not Good! Know Why? under the section, Adaptive Code.

If that's indeed the case, I may have to rethink this approach as it appears to be resulting in some confusing behaviour. You see, I don't want a misstep in using the Ads library to result in an 'error' in production, but I do want the developer to be aware of such a misstep while in development.

Please, confirm you're instantiating the Ads class only once in the app. You may have to instantiate it to a static variable in a 'wrapper class' if you want to then have ready access to that library package throughout your app.

Regards,

Greg.

SirJohn2024 commented 4 years ago

Thanks for the response, but I think the missing appId is not the problem…

In the sample app main.dart that I sent you, the appId parameter is actually passed in the initState() method (there was a typo at line 126 in the file I sent, see attached file):

ads = Ads(adsAppId,

    testing: debugBuild,

    listener: adsEventListener,

    bannerUnitId: adsUnitId);

My sample code has the static parameter adsAppId obfuscated...

static var adsAppId = Platform.isAndroid

  ? "ca-app-pub-YYYYYYYYYYYYYYYY~XXXXXXXXXX"

  : "ca-app-pub-YYYYYYYYYYYYYYYY/XXXXXXXXXX";

Seems legit to me…

What do you think?

From: Andrious Solutions notifications@github.com Sent: sabato 19 ottobre 2019 15:00 To: AndriousSolutions/ads ads@noreply.github.com Cc: Sergio sergio.bassan.development@gmail.com; Author author@noreply.github.com Subject: Re: [AndriousSolutions/ads] New versions display only test ads in release mode... (#17)

In fact, looking further into the "invalid App id" issue, that would be the reason why you're getting 'test ads' in production. There's no App id supplied to the constructor. Further, in these most recent versions of Ads, you can instantiate only one instance of the Ads object. However, there's no Assert statement to warn you of this. The app will merely not display any ads. The screenshot of the constructor below shows you how a test app id is instead used if no other is supplied: https://user-images.githubusercontent.com/32497443/67145229-4ffa9c00-f245-11e9-955d-26e097cea8ee.png Possibly this is what you are witnessing in your app. It's explained further in my article, My Code’s Not Good! Know Why? https://medium.com/@greg.perry/my-apps-no-good-here-s-why-7699347bb235 under the section, Adaptive Code https://medium.com/@greg.perry/my-apps-no-good-here-s-why-7699347bb235#5e80 .

If that's indeed the case, I may have to rethink this approach as it appears to be resulting in some confusing behaviour. You see, I don't want a misstep in using the Ads library to result in an 'error' in production, but I do want the developer to be aware of such a misstep while in development.

Please, confirm you're instantiating the Ads class only once in the app. You may have to instantiate it to a static variable in a 'wrapper class' if you want to then have ready access to that library package throughout your app.

Regards,

Greg.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/AndriousSolutions/ads/issues/17?email_source=notifications&email_token=AABTS2KAR4F2GZQOTCCAXGDQPMAFDA5CNFSM4I7ZD2R2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBXO7GY#issuecomment-544141211 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AABTS2ISHZRTIJW7VN2EIULQPMAFDANCNFSM4I7ZD2RQ . https://github.com/notifications/beacon/AABTS2L22HUU7ZDPPRENDCDQPMAFDA5CNFSM4I7ZD2R2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBXO7GY.gif

aydinfatih commented 4 years ago

In my app, continuous test ads appear. When I check my Admob account, the match rate is 1,20%. Why so low? Is there a problem with the plugin?

Andrious commented 4 years ago

@SirJohn2024 I would think that sample code is just fine.

I'll set aside time today to again perform tests on my own 'test phones.' In the meantime, another individual has described such 'odd behaviour' as well, and although this only works on the 'development side', I will have to ask you all to 'look under the hood' and confirm you're code is supplying the appropriate id's to the library package.

Please, go to your source code and place some 'breakpoints' on line 254, 557 and 568 in the Ads package pictured below to confirm you're getting your id's and not 'test id's' being assigned to the plugin. Other's using this Ads package is not getting the issues you're having, you see. I'm just confirming we've checked every possibly. It's such odd behaviour, if I can not find anything, we may have to contact the authors of the plugin, firebase_admob, and see if they will be able to resolve this strange issue. InterstitialAdCode RewardedVideoAdCode @nemoryoliver

Greg

adithyaxx commented 4 years ago

I had the same issue and fixed it by supplying the bannerId to showBanner as adUnitId. I am not sure if this is the intended behaviour but it seems redundant to have to supply the id again in spite of having initialised the class once with it. Regardless, I will submit a PR to workaround this issue and see what @Andrious thinks.

SirJohn2024 commented 4 years ago

Thanks for the tip... It works...

Andrious commented 4 years ago

Well, at least we're getting somewhere. Let me get this straight. You're supplying the bannerUnitId twice, and that displays a 'production' ad and not a 'test' ad?!

hehe Now, that is very strange. It invokes more questions than answers.

Please, adithyaxx, you would supply me the code doing just that. I'm glad it works, but it should work in the first place.

You see, supplying the 'bannerUnitId' both when initializing and then when showing the ad merely disposes the first initialized ad from memory then reloads the ad into memory again when it's finally shown.

Now that doesn't make sense does it!? The plugin is not behaving as expected.

I'll certainly look into this.

Andrious commented 4 years ago

Again, the strange thing is it's working fine for me and, as far as I know, for most of those using this library package.

Let's do some experiments and see if we can't pin this down.

Since it's been found that a 'fix' involves supplying again the id originally supplied to the constructor's parameter, bannerUnitId, to the show function's parameter, adUnitId, let's see what happens if you don't supply the id to the constructor in the first place.

Don't bother with the parameter, bannerUnitId, in the constructor at all and just supply the ID to the parameter, adUnitId, in the show function.

I know it sounds trivial, but let's see if that resolves the issue. I've seen stranger things. I need more to go on. There's no reason that you're tip should have resolved the issue you see. From my point of view as the author and maintainer of the library package, it, in fact, compounds the issue.

There's no point for me to try this experiment. I mean it already works fine for me. There's something we're missing. Something 'that's different' from my situation to your situation. Welcome to the wonderful World of programming.

Cheers.

adithyaxx commented 4 years ago

Don't bother with the parameter, bannerUnitId, in the constructor at all and just supply the ID to the parameter, adUnitId, in the show function.

Tried this and I am able to get production ads this way. So it seems like the bannerId is only used when passed to the show function. At least for banner ads, passing the bannerId to the constructor does not have any visible effect.

Andrious commented 4 years ago

Well, now that is interesting.

But it does have a visible effect, does it not? If you had instead supplied the ID to the constructor, you would 'consistently' get 'test' ads and not 'production' ads. Is that not the case?

Regardless, I'll walk through the code in the hope that I can see what the problem may be.

It's a subtle bug, but I'll see if I can recreate the issue and possibly resolve the it. I mean, by design, when the id is passed to the constructor, it should produce production ads! I'll take a look later in the day.

Thanks.

Andrious commented 4 years ago

That one little experiment on your part allowed me to walk through the code and look at it 'from a different angle' as it were. With that, I feel the issue has been fixed with the latest version, 1.2.1, that's now available.

Do review the package website's README file to ensure it's implemented correctly, but I feel the it now working as designed. Specifically, if you've supplied the ID's to the constructor and not to the show() (or vice versa!) you will receive your 'production' ads and not 'test' ads.

Andrious commented 4 years ago

@SirJohn2024 As you've implied, the library package was lacking some means to notify developers using the package that the Ads have failed to load into memory or have failed be shown and displayed properly for one reason or another.

It's a valuable characteristic, and with version 1.3.0, a value of Future\<bool> is returned when an Ad is 'set' or is 'shown' allowing developers to determine if such operations were successful or not. Both operations are asynchronous and hence involve Future's, but regardless, you now have a means to implement this library package in your code and be notified if something goes wrong.

This library package works with the third-party plugin, firebase_admob. Meaning it has to adapt if the plugin fails for some unknown reason. Regardless, the developer needs to be notified of the failure and response accordingly depending on their app's particular situation.

Further, as you had also mentioned, at the core of these recent changes to the library package, there is the act of 'trying it one more time' if the ads fails to be shown---for some unknown reason regarding the plugin. The screenshot below demonstrates this second attempt. If this second attempt then fails, the error is recorded and the developer notified with a Future value of false. loadAgain

The screenshot below demonstrates how a developer can, for example, set a Banner Ad in their code and determine if the operation failed to set---returning a boolean value of false. It further demonstrates how the developer could use the new property, inError, to determine if error actually occurred. Lastly, with the new function, getError(), the developer can even retrieve that error. The second example below merely demonstrates the same thing with the 'then' clause. bannerError

As you see in the screenshot below, there's three more getters and three more functions allowing the developer to determine if the Ads failed, in this case, to show properly. As you see, you can determine and get errors for the specific Ads. showError

I will be releasing this latest version later in the day, Friday.

Greg