search

lkorth / php-gcm

An easy to use gcm library for PHP
Apache License 2.0
180 stars 45 forks source link

GCM Group Notifications #16

Open vitorhorta opened 8 years ago

vitorhorta commented 8 years ago

Hi! First of all, thanks for the great package.

I would like to know if you are planning to support registering and sending messages to GCM Group.

As you can see here: https://developers.google.com/cloud-messaging/notifications to support Group messages we need to be able to:

  1. Register to a group
  2. Add/Remove devices to groups
  3. Send message to group
lkorth commented 8 years ago

I just finished adding initial support for device group messaging in f1b3161ba8e121680b59d220b904bf19ba90abdc. I would appreciate it if you could test it out and provide any feedback or issues you may encounter.

vitorhorta commented 8 years ago

I've just checked the code and it looks great! I couldn't test it yet but I'll do it soon (probably tomorrow) and give you a better feedback.

vitorhorta commented 8 years ago

It's working great, thanks for the enhancement. I only have few questions.

1- Are you implementing the send method for group operations? 2- Shouldn't we make the SENDER_ID an attribute of the Sender class? 3- GCM sometimes returns 200 but the operation is not fully successful. For example, when we try to create a group that already exists we get a HTTP 200 code and a message: ''{"error":"notification_key already exists"}'". There are similar cases like this one. Should we catch these "errors" ?

Please let me know if you need any help with these improvements!

lkorth commented 8 years ago
  1. Yes, the responses are different and are not parsed correctly right now. Either the send method needs to parse either possible response or there needs to be a separate send method.
  2. Yes, I think that makes more sense in the long run.
  3. I ran across this when adding support for groups, but I didn't notice that they were 200 responses. It's unfortunate that part of their API respects status codes and part of it does not. The only solution would probably be to check for the presence of the top level error key when deciding if the request was a success or not.