srcfl / srcful-gateway

Sourceful Energy Gateway: Connect your solar inverter or Utility meters to earn tokens.
https://sourceful.energy/
MIT License
5 stars 2 forks source link

Rewrite the bluetooth part of the GW #67

Closed davmoz closed 10 months ago

davmoz commented 10 months ago

I suggest a complete rewrite of the bluetooth service. The new version should define characteristics for each value the eGW exposes for read/write or both. This will make things easier (if documented properly) for anyone that want to build IOS/Android apps for us in the future!

https://chat.openai.com/share/33d5c77a-2074-4986-afb0-71880fa54db5

Suggested structure (3 services, each with its characteristics):

# WiFi Service
UUID_WIFI_SERVICE = '2de3f007-4404-4d72-982d-f6ec41eed4d2'
# WiFi Characteristics UUIDs
UUID_WIFI_SSID = '505ff120-cc05-4919-9878-62e4b8c166ae'
UUID_WIFI_PSK = '8361f5c3-af26-48e5-a5aa-09ef5a5ff02b'
UUID_WIFI_IS_CONNECTED = 'fa195042-c70e-49eb-87c7-6e328a7d0f4d'

# Wallet Service
UUID_WALLET_SERVICE = 'd1b64f27-fa90-46f5-84e3-8a89f0687f5e'
# Wallet Characteristics UUIDs
UUID_WALLET_ADDRESS = '26a87e77-ecb1-497c-ab0a-25f757c80848'
UUID_WALLET_IS_REGISTERED = '3502615e-fa20-4a07-a95a-4a83cd1941de'

# Inverter Service
UUID_INVERTER_SERVICE = '1af94563-7e8b-47f2-9748-f0b6bd35a621'
# Inverter Characteristics UUIDs
UUID_INVERTER_IP = '938af5da-6ba8-4876-9a5e-4abfd784817b'
UUID_INVERTER_PORT = 'bdee067a-6d5e-4534-8907-f42ec2aacaa8'
UUID_INVERTER_DEVICE_ADDRESS = '3c0e0141-86ca-4e59-abd1-f03c440483ed'
UUID_INVERTER_TYPE = 'c6ad331f-4d5a-432e-8561-020784a68636'
UUID_INVERTER_CONFIG_SAVED = '191b107b-fbbd-47b1-bd50-760fb32ecb97'
UUID_INVERTER_IS_COMMUNICATING = 'b72be81b-9da8-4cf5-b30b-856e79266fcf'

gatt: Dict = {
    # WiFi Service
    UUID_WIFI_SERVICE: {
        # SSID (Writable)
        UUID_WIFI_SSID: {
            "Properties": GATTCharacteristicProperties.write,
            "Permissions": GATTAttributePermissions.writeable,
            "Value": None
        },
        # PSK (Writable)
        UUID_WIFI_PSK: {
            "Properties": GATTCharacteristicProperties.write,
            "Permissions": GATTAttributePermissions.writeable,
            "Value": None
        },
        # isConnected (Readable)
        UUID_WIFI_IS_CONNECTED: {
            "Properties": GATTCharacteristicProperties.read,
            "Permissions": GATTAttributePermissions.readable,
            "Value": None
        }
    },
    # Wallet Service
    UUID_WALLET_SERVICE: {
        # Wallet Address (Writable)
        UUID_WALLET_ADDRESS: {
            "Properties": GATTCharacteristicProperties.write,
            "Permissions": GATTAttributePermissions.writeable,
            "Value": None
        },
        # isRegistered (Readable)
        UUID_WALLET_IS_REGISTERED: {
            "Properties": GATTCharacteristicProperties.read,
            "Permissions": GATTAttributePermissions.readable,
            "Value": None
        }
    },
    # Inverter Service
    UUID_INVERTER_SERVICE: {
        # IP (Writable)
        UUID_INVERTER_IP: {
            "Properties": GATTCharacteristicProperties.write,
            "Permissions": GATTAttributePermissions.writeable,
            "Value": None
        },
        # Port (Writable)
        UUID_INVERTER_PORT: {
            "Properties": GATTCharacteristicProperties.write,
            "Permissions": GATTAttributePermissions.writeable,
            "Value": None
        },
        # DeviceAddress (Writable)
        UUID_INVERTER_DEVICE_ADDRESS: {
            "Properties": GATTCharacteristicProperties.write,
            "Permissions": GATTAttributePermissions.writeable,
            "Value": None
        },
        # type (Writable)
        UUID_INVERTER_TYPE: {
            "Properties": GATTCharacteristicProperties.write,
            "Permissions": GATTAttributePermissions.writeable,
            "Value": None
        },
        # configSaved (Readable)
        UUID_INVERTER_CONFIG_SAVED: {
            "Properties": GATTCharacteristicProperties.read,
            "Permissions": GATTAttributePermissions.readable,
            "Value": None
        },
        # isCommunicating (Readable)
        UUID_INVERTER_IS_COMMUNICATING: {
            "Properties": GATTCharacteristicProperties.read,
            "Permissions": GATTAttributePermissions.readable,
            "Value": None
        }
    },
}

Need your feedback, @tobias-dv-lnu @kneeclass!

h0bb3 commented 10 months ago

I don't really see the value in this. The current design works and has the advantage that the ble thing is just a proxy that reroutes to the gw API. This has many advantages as we do not need double documentation, double apis, and we do not need to change the ble client to add new functionality.

It should be perfectly fine to write any ios/android apps on top of this?

What is the reason for you wanting to change this?

h0bb3 commented 10 months ago

The only technical downside with the current approach as I see it is that we are somewhat wasteful in what is sent/recieved i.e. we are sending what is essentially REST requests (a lot of text) dedicated characteristics would avoid this. But it is also not that we are in need of realtime data streaming over ble for the stuff we want to do.

The clients will also need to handle a bit more responsibility in message formatting and message queueing - but this really pretty basic stuff.

davmoz commented 10 months ago

This has many advantages as we do not need double documentation, double apis, and we do not need to change the ble client to add new functionality.

I agree regarding the documentation part, but I think we need to change the client either way.. How are we going to display new values if not adding the new end-point (or service/characteristic(s)) to the client and display incoming values?

I personally don't like the idea of components guessing what values are available in the characteristic. If component A asks for data at the same time (?) as component B, then both will end up filtering the response for their own gain. E.g when connected to the eGW, we want to display eGW status (uptime, internet connection, inverter communication, active wallet etc..) at the same time as sending POST-requests to configure (say) the inverter.

What component will be responsible for delegating responses to their respective components? This is probably my incompetence of react, but how do we do things the clean way here?

The only technical downside with the current approach as I see it is that we are somewhat wasteful in what is sent/received i.e. we are sending what is essentially REST requests (a lot of text) dedicated characteristics would avoid this. But it is also not that we are in need of realtime data streaming over BLE for the stuff we want to do.

The maximum transmittable size in a characteristic seems to be 512 bytes (3.2.9. Long attribute values). This does not cause any issues at the moment, but is a restriction that we will have to live with if we continue this path. Do we still need to do an MVP here or do we want to create something robust while we are at it?

h0bb3 commented 10 months ago

How are we going to display new values if not add the new end-point (or service/characteristic(s)) to the client and display incoming values? I meant the ble service (I call it a client as it is the client of the server). You of course need to provide UI stuff of the data.

I personally don't like the idea of components guessing what values are available in the characteristic. If component A asks for data at the same time (?) as component B, then both will end up filtering the response for their own gain. E.g when connected to the eGW, we want to display eGW status (uptime, internet connection, inverter communication, active wallet etc..) at the same time as sending POST-requests to configure (say) the inverter.

You implement a message queue, responses are labeled with the sending endpoint (you could probably even pair these so that you don't overwrite a response with a request). All of this is already done in the current client (not request response pairing), you can just use it as a js module (some refactoring needed). It would be appropriate to let the top-level ble component be responsible for the queue and change state of sub components that are interested in certain messages.

There is no guessing of things that are sent - you get json rest responses. If you go the characteristics way you would need custom parsing of every message - 4 bytes, interpreted to something else big small endian etc. Yes more optimal in transmission size but a pain to work with and sensitive and slow in adding new features as the ble service would also need change and a protocol - we will probably invent something there - maybe sending everything as strings - presto HTTP.

This will be slow and error-prone process.

If someone on the outside gets an idea of making something it would essentially mean that we would need to implement it in the ble service, and push it to all devices. Even for the endpoints we already have implemented.

Do you plan on writing a service with X characteristics for each currently implemented endpoint? You would need 16 services with at least 2 characteristics each (more for posts) just to cover what we have today... so we are talking about 40 characteristics... this would be infeasible imho.

Why do you think using separate characteristics will be more robust? There will be a lot more room for errors and mistakes. I mean implementing 40 places that will recieve/send custom data?

The most optimal way would be to just use ble for wifi config and then just send the IP as the response. The client should then switch to this IP and use that. The problem with this approach is that the receiving unit must be on the same network as the gw and that is maybe not always the case (some ppl probably run the gw in their own sub-nets). So I guess the current basic config is some type of middle ground.

I would say that a special service/characteristic would be needed if we are to stream data - but this a far cry from what we need ble for today. The server itself does not support streaming of data atm so this would be a major thing to implement anyway.

Yes I'm aware of the potential limit of 512 bytes - but this has not been confirmed to be a problem and we could take some measures to first test if this actually is a limit. Calling GET api/network would be a good test (at least on a rpi as there response is quite large). if it is a problem then there are some measures to take that are not too invasive (e.g. URL shortening/lookup)

It could be an idea to separate request and response into two separate characteristics. I don't know if it is strictly needed but it could be a thing that may simplify the client to some degree.