shamblett / coap

A Coap package for dart
Other
16 stars 13 forks source link

Since version 4.2.0 observe non confirmable not working properly #107

Closed vincent-iQontrol closed 1 year ago

vincent-iQontrol commented 1 year ago

I use a non confirmable observer from the example. Version 4.0.0 and 4.1.0 is working correctly, all events are coming in. In 4.2.0 and above a lot of events are missing after ~ 13 events.

final reqObsNon = CoapRequest(CoapCode.get, confirmable: false)..addUriPath('Json');

shamblett commented 1 year ago

@JosefWN and @JKRhb any thoughts?

JosefWN commented 1 year ago

Confirmed using this snippet (works in 4.1.0 but not in 4.2.0):

import 'dart:async';
import 'package:coap/coap.dart';
import 'config/coap_config.dart';

FutureOr main() async {
  final conf = CoapConfig();
  final uri = Uri(
    scheme: 'coap',
    host: 'californium.eclipseprojects.io',
    port: conf.defaultPort,
  );
  final client = CoapClient(uri, conf);

  try {
    final reqObsNon = CoapRequest(CoapCode.get, confirmable: false);
    reqObsNon.addUriPath('obs-non');

    print('Observing /obs-non on ${uri.host}');
    final obsNon = await client.observe(reqObsNon);
    obsNon.stream.listen((CoapRespondEvent e) {
      print('/obs-non response: ${e.resp.payloadString}');
    });

    await Future.delayed(Duration(minutes: 15));
  } catch (e) {
    print('CoAP encountered an exception: $e');
  }

  client.close();
}

I will try to dig into it soon.

vincent-iQontrol commented 1 year ago

Thanks for investigating. I will use an older version for the time being.

JosefWN commented 1 year ago

Made a couple of more tests, this bug seems to have been introduced with my stricter linting of the repo...

git checkout 56508455fc0624f9b831fd87024907013e228584
dart get_observe_async.dart
Observing /obs-non on californium.eclipseprojects.io
/obs-non response: 15:47:44
/obs-non response: 15:47:49
/obs-non response: 15:47:54
Unhandled exception:
Null check operator used on a null value
#0      CoapEndPoint._serializeEmpty (package:coap/src/net/coap_endpoint.dart:240:17)
#1      CoapEndPoint.sendEmptyMessage (package:coap/src/net/coap_endpoint.dart:221:20)
#2      CoapStackBottomLayer.sendEmptyMessage (package:coap/src/stack/coap_layer_stack.dart:171:29)
#3      CoapNextLayer.sendEmptyMessage (package:coap/src/stack/coap_layer_stack.dart:44:10)
#4      CoapAbstractLayer.sendEmptyMessage (package:coap/src/stack/coap_abstract_layer.dart:44:15)
#5      CoapReliabilityLayer.receiveResponse (package:coap/src/stack/coap_reliability_layer.dart:224:7)
#6      CoapNextLayer.receiveResponse (package:coap/src/stack/coap_layer_stack.dart:59:10)
#7      CoapAbstractLayer.receiveResponse (package:coap/src/stack/coap_abstract_layer.dart:62:15)
#8      CoapLayerStack.receiveResponse (package:coap/src/stack/coap_layer_stack.dart:214:18)
#9      CoapEndPoint._receiveData (package:coap/src/net/coap_endpoint.dart:163:22)
#10     _RootZone.runUnaryGuarded (dart:async/zone.dart:1618:10)
#11     CastStreamSubscription._onData (dart:_internal/async_cast.dart:85:11)
JosefWN commented 1 year ago

Anyway, let's try and get the fixes we have pending merged then work from there.

shamblett commented 1 year ago

Yep ok Ill look at the PRs over the weekend ta.

shamblett commented 1 year ago

With this and #112 now merged are we ready for another release? There's quite a bitt gone since 4.2.1

JosefWN commented 1 year ago

We could merge PR #111 too perhaps? Then I can rebase my other PR on master to reduce its size.

JKRhb commented 1 year ago

With this and #112 now merged are we ready for another release? There's quite a bitt gone since 4.2.1

I also have one or two more upcoming PRs which I will open soon, then we could go for a new release :)

shamblett commented 1 year ago

OK PR #111 merged, I'll hold off on the release.

JKRhb commented 1 year ago

I also have one or two more upcoming PRs which I will open soon, then we could go for a new release :)

Okay, I think I've now opened all PRs I had in flight :) Due to the fact that some of our PRs contain breaking changes, should we go for a 5.0.0 release?

shamblett commented 1 year ago

Believe this has been addressed. Package re released at version 5.0.0.

shamblett commented 1 year ago

Package now re published at version 6.0.0, closing this issue.