Skip to content

Conversation

cloudwebrtc
Copy link
Contributor

@cloudwebrtc cloudwebrtc commented Jul 18, 2022

Since the SDK uses the Enum.name feature, it is only downgraded to dart 2.15.0, and the corresponding flutter version is 2.8.x
Compilation tests passed in flutter 2.8.1 and 3.0.2.

Copy link
Member

@hiroshihorie hiroshihorie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!
it's not hard to implement Enum.name if we need even lower support.
But should we support lower than 2.8.1 ?

@cloudwebrtc
Copy link
Contributor Author

cloudwebrtc commented Jul 18, 2022

yeah, we can try downgrading to a lower version.
and flutter-webrtc can support 2.0.0+

@cloudwebrtc
Copy link
Contributor Author

cloudwebrtc commented Jul 18, 2022

But should we support lower than 2.8.1 ?

@hiroshihorie I did a more detailed investigation and I tried to downgrade the version to 2.0.0, but failed,
There are several places that need to modify multiple codes

  • Enum.name
  • Object.hash

These two features are only supported by dart 2.15.0+. If these places are modified for downgrading, the code readability will be reduced, so I think keeping the current 2.8.0+ support, we can still span three major versions. 2.8.x, 2.10.x and 3.0.x
so, what do you think?

Downgrading to 2.0.0 gives the following error.

lib/pages/connect.dart:259:33: Error: No named parameter with the name 'color'.
                                color: Colors.white,                    
                                ^^^^^                                   
../../../../fvm/versions/2.0.0/packages/flutter/lib/src/material/progress_indicator.dart:542:9: Context: Found this candidate, but the arguments don't match.
  const CircularProgressIndicator({                                     
        ^^^^^^^^^^^^^^^^^^^^^^^^^                                       
lib/exts.dart:165:35: Error: The getter 'name' isn't defined for the class 'SimulateScenarioResult'.
 - 'SimulateScenarioResult' is from 'package:livekit_example/exts.dart' ('lib/exts.dart').
Try correcting the name to the name of an existing getter, or defining a getter or field named 'name'.
                    child: Text(e.name),                                
                                  ^^^^                                  
../lib/src/types/video_dimensions.dart:39:30: Error: Method not found: 'Object.hash'.
  int get hashCode => Object.hash(width, height);                       
                             ^^^^                                       
../lib/src/types/video_encoding.dart:30:30: Error: Method not found: 'Object.hash'.
  int get hashCode => Object.hash(maxFramerate, maxBitrate);            
                             ^^^^                                       
../lib/src/types/video_parameters.dart:30:30: Error: Method not found: 'Object.hash'.
  int get hashCode => Object.hash(description, dimensions, encoding);   
                             ^^^^                                       
../lib/src/core/signal_client.dart:411:29: Error: The getter 'name' isn't defined for the class 'ConnectionState'.
 - 'ConnectionState' is from 'package:livekit_client/src/types/other.dart' ('../lib/src/types/other.dart').
Try correcting the name to the name of an existing getter, or defining a getter or field named 'name'.
        '${_connectionState.name} -> ${newValue.name}');                
                            ^^^^                                        
../lib/src/core/signal_client.dart:411:49: Error: The getter 'name' isn't defined for the class 'ConnectionState'.
 - 'ConnectionState' is from 'package:livekit_client/src/types/other.dart' ('../lib/src/types/other.dart').
Try correcting the name to the name of an existing getter, or defining a getter or field named 'name'.
        '${_connectionState.name} -> ${newValue.name}');                
                                                ^^^^                    
../lib/src/internal/events.dart:118:59: Error: The getter 'name' isn't defined for the class 'ConnectionState'.
 - 'ConnectionState' is from 'package:livekit_client/src/types/other.dart' ('../lib/src/types/other.dart').
Try correcting the name to the name of an existing getter, or defining a getter or field named 'name'.
  String toString() => '$runtimeType(newState: ${newState.name}, '      
                                                          ^^^^          
../lib/src/core/engine.dart:540:73: Error: The getter 'name' isn't defined for the class 'DisconnectReason'.
 - 'DisconnectReason' is from 'package:livekit_client/src/types/other.dart' ('../lib/src/types/other.dart').
Try correcting the name to the name of an existing getter, or defining a getter or field named 'name'.
        .info('onDisconnected state:${_connectionState} reason:${reason.name}');
                                                                        ^^^^
../lib/src/core/engine.dart:670:29: Error: The getter 'name' isn't defined for the class 'ConnectionState'.
 - 'ConnectionState' is from 'package:livekit_client/src/types/other.dart' ('../lib/src/types/other.dart').
Try correcting the name to the name of an existing getter, or defining a getter or field named 'name'.
        '${_connectionState.name} -> ${newValue.name}');                
                            ^^^^                                        
../lib/src/core/engine.dart:670:49: Error: The getter 'name' isn't defined for the class 'ConnectionState'.
 - 'ConnectionState' is from 'package:livekit_client/src/types/other.dart' ('../lib/src/types/other.dart').
Try correcting the name to the name of an existing getter, or defining a getter or field named 'name'.
        '${_connectionState.name} -> ${newValue.name}');                
                                                ^^^^                    
../lib/src/utils.dart:81:37: Error: The getter 'name' isn't defined for the class 'TargetPlatform'.
 - 'TargetPlatform' is from 'package:flutter/src/foundation/platform.dart' ('../../../../fvm/versions/2.0.0/packages/flutter/lib/src/foundation/platform.dart').
Try correcting the name to the name of an existing getter, or defining a getter or field named 'name'.
          os: defaultTargetPlatform.name,       

@cloudwebrtc
Copy link
Contributor Author

I made some code modifications to make our SDK work in each version of flutter 2.0.0~3.0.5

I can compile and run normally in the following sdk versions.

duanweiwei@duanweiMBPIntel client-sdk-flutter % fvm list
fvm: 2.0.0
fvm: 2.10.5 (current)
fvm: 2.2.0
fvm: 2.5.3
fvm: 2.8.1
fvm: 3.0.0
fvm: 3.0.5

EnumObj.name

+String enumName(Object o) => o.toString().split('.').last;
...
-        '${_connectionState.name} -> ${newValue.name}');
+        '${enumName(_connectionState)} -> ${enumName(newValue)}');

Object.hash

-  int get hashCode => Object.hash(maxFramerate, maxBitrate);
+  int get hashCode => identityHashCode(identityHashCode(maxFramerate) + identityHashCode(maxBitrate));

I haven't submitted this commit yet, I think it's worthwhile to exchange this small code modification for the wide-range version support of the SDK, @hiroshihorie @davidzhao what do you think.

@davidzhao
Copy link
Member

I'm all for it. Just curious, are users asking for supporting older flutter versions? How many people are still using 2.8.0?

I think unlike Device OS support, it seems pretty easy to update Flutter version for the developer right?

@cloudwebrtc
Copy link
Contributor Author

I'm all for it. Just curious, are users asking for supporting older flutter versions? How many people are still using 2.8.0?
I think unlike Device OS support, it seems pretty easy to update Flutter version for the developer right?

yes, I think so.

The minimum version of flutter that supports m1 mac is flutter 3.0.0, only intel mac or windows/linux can support flutter 2.x, and 2.8.0 was released on 2021/12/9, so from my developer's point of view, there are 6 Months also seems to be enough (no need to reverse update), so I merge to 2.8.0 support first (don't submit downgrade to 2.0.0 related patches to avoid some dirty code)

@cloudwebrtc cloudwebrtc merged commit c772271 into main Jul 18, 2022
@cloudwebrtc cloudwebrtc deleted the downgrade-deps-versions-to-support-flutter-2.8.x+ branch July 18, 2022 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants