jekalmin / extended_openai_conversation

Home Assistant custom component of conversation agent. It uses OpenAI to control your devices.
834 stars 108 forks source link

User user_id for user identification #165

Closed danielp370 closed 3 months ago

danielp370 commented 4 months ago

I have the Voice over IP integration working with extended OpenAI Conversation, which allows me to VoIP connect into the voice assistant. However, to make this work we need to pass an acceptable user.name to OpenAI which does not allow spaces.

Currently this is the error we receive:

Error code: 400 - {'error': {'message': "'Voice over IP' does not match '^[a-zA-Z0-9_-]{1,64}$' - 'messages.1.name'", 'type': 'invalid_request_error', 'param': None, 'code': None}}

The attached patch converts spaces to underscores, and the VoIP integration now works well with this integration.

danielp370 commented 4 months ago

If you want to try it out see https://community.home-assistant.io/t/voice-over-ip-integration-call-from-any-sip-softphone/567762

jekalmin commented 3 months ago

Thanks for your contribution!

I was planning to use other field instead of friendly_name. (see https://github.com/jekalmin/extended_openai_conversation/issues/63) If the constraint of Voice over IP is ^[a-zA-Z0-9_-]{1,64}$, I would probably go for user_id of attributes.

If you don't mind, could make a change? so that user_id(hash) is used.

danielp370 commented 3 months ago

Will do - I think the constraint is from open AI. Open AI say this: "The name of the author of this message. May contain a-z, A-Z, 0-9, and underscores, with a maximum length of 64 characters."

danielp370 commented 3 months ago

hi @jekalmin I've made the changes to pass in user_id hash. In a separate commit I also added a function called get_user_from_user_id with a spec readme.md. I've tested all this for the Voice Over IP integration, as well as user's with special characters in their names in #63 and it works well. Btw, I dont think we could rely on the alternative proposal of using entity_id as not all entities that will converse with this integration are person.xxx, and it still violates the character rules for openAI names.

image
jekalmin commented 3 months ago

Thanks for your contribution! I just tested and it works great!

One concern is that it exposes private data such as access_token, refresh_token, jwt_key, and etc... I think it would be nice to only expose what we need such as name or others.

How about returning simple dict like {'name': 'YOUR_NAME'}, so that we can make it possible to add other fields later?

danielp370 commented 3 months ago

Fixed thanks, sorry I missed that! I checked if any other fields may be useful to add to the dictionary now, however I couldn't think of anything useful to add at this time.

jekalmin commented 3 months ago

I agree that currently, name is the only field we need. Thanks for your contribution again!