Should this be upstreamed into esp-idf-svc?

[svenstaro]

Hey, great project! We made our own implementation of essentially this a while back and would like to upstream it back to esp-idf-svc but then I stumbled upon this project right here. The question now is: Are you interested in upstreaming this or do you plan to keep this separate?

I think there's value in putting this into esp-idf-svc if it's specific to ESP32 anyway.

[persello]

Absolutely, I'd love to help with the development of esp-idf-svc. I don't have much free time until this summer, but I can still help.

[persello]

But before upstreaming it, I'd like to let you know about some quirks and todos that I don't have the time to fix right now.

[svenstaro]

Do you currently handle packet fragmentation? That is, how do you currently handle sending of packets larger than MTU?

[persello]

Honestly, I have never tested this. I didn’t do anything to explicitly support it, but, from the original bluedroid documentation, I see that the value field of an esp_gatt_value_t struct shall have a maximum length of ESP_GATT_MAX_ATTR_LEN.

The code that handles responses is available here.

I didn’t have much time to check the documentation, but I haven't found any references to queued operations.

[svenstaro]

We're working on upstreaming this combined with our code. Would you like to work with us directly in some fashion? We could set up a meeting (CET) if you'd like to talk. If you're interested, write me a mail. :)

[cwoolum]

I have a working implementation of chunking. I can wait until the current code is up streamed though to PR it to avoid too much more churn.

[svenstaro]

@cwoolum by chunking you mean fragmentation? That is, being able to re-assemble payloads with > MTY size directly in the Bluetooth stack?

[cwoolum]

Sorry, yes! At this time, it always assumes that the MTU is 22 but it can be modified in the future to take MTU update events into account.

[svenstaro]

That's great! Yes, that'd be an amazing contribution. Let's not put too much on upstream's table though. I think the current PR is a good basis that we can add stuff to. You could already prepare a follow-up, if you like.

[cwoolum]

Well, the other issue is that I can't use the latest version of this package due to versioning issues breaking me locally in #17 . I want to have everything working before I submit a PR. I can try digging into that next.

Which current PR are you referring to?

[svenstaro]

We're upstreaming it here: esp-rs/esp-idf-svc#247

We're also using IDF 4.4 so it's odd that it's broken for you.

[ivmarkov]

I'm quite busy this week so realistically I can provide feedback on the PR will next week. To set the expectations - some changes - mainly de-stdfication as esp-idf-svc is actually avoiding the std lib where possible - as well as a change of the gatt server singleton pattern (it should be based on top of the modem peripheral singleton) are to be expected. Nothing major, but to give an early signal that I would need someone to help me with applying those changes to the PR.