Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some return types are not public #7

Closed
mullerch opened this issue Dec 4, 2023 · 4 comments · Fixed by #8
Closed

Some return types are not public #7

mullerch opened this issue Dec 4, 2023 · 4 comments · Fixed by #8

Comments

@mullerch
Copy link
Contributor

mullerch commented Dec 4, 2023

Some public functions, such as Modem3gpp.Scan() return private types (network3Gpp). Those types have no functions that can ingest them to return useful public types. Those functions seem therefore useless.

I don't see a reason for the following types (non-exhaustive) to be made private:

  • network3Gpp
  • networkScanResult
  • rawPcoData
  • port
  • bearerIpConfig
  • bearerStats

Is this visibility intentional? If yes, how are we supposed to use the output of those functions?

@maltegrosse
Copy link
Owner

maltegrosse commented Dec 4, 2023

Hi @mullerch
Its quite a long time ago I did this implementation, but I try my best to explain it (as far as I remember)

You can get the values as following: (ignored errors by _)

mmgr,_ := modemmanager.NewModemManager()
modems, _ := mmgr.GetModems()
for _,m :=range modems{
	ports, _ := m.GetPorts()
	for _, p := range ports{
		fmt.Println(p.PortName)
	}
	g, _ := m.Get3gpp()
	g.RequestScan()
	sc,_ := g.Scan()
	for _,scans := range sc{
		fmt.Println(scans.Mcc)
	}
	res, _ := g.GetScanResults()
	
	for _, net := range res.Networks{
		fmt.Println(net.Mcc)
	}
	rawPco ,_ := g.GetPco()
	fmt.Println(rawPco)
	
	bearer, _ :=g.GetInitialEpsBearer()
	ip, _ := bearer.GetIp4Config()
	fmt.Println(ip.Address)

	stats,_ := bearer.GetStats()
	fmt.Println(stats)
	
}

Most of the structs are just helper structs,in order to access the raw dbus values in a more convenient way.

Getting the different values can caus multiple errors on dbus side + parsing etc. The functions are created to make the user aware of these errors. In my opinion there is no need to make the structs public, as their properties are public and functions to access them exist. (and the users can not do anything with these structs on their own...)

Let me know if I am wrong and any improvements/PRs are welcome

Additionally, a basic example exist here: https://github.com/maltegrosse/go-modemmanager/blob/master/examples/test_run.go

@mullerch
Copy link
Contributor Author

mullerch commented Dec 5, 2023

Thanks for your quick answer.

With private visibility of the type, the library user cannot:

  1. Declare a variable of that type
  2. Use the type as parameter / return type of a function
  3. Instantiate the type

1 & 2 impose design constraints to the user's code, which is weird (e.g. you cannot create a wrapper function without abstracting the type). I don't see any reason why one may need 3.

I suggest making those types public so that there are no such constraints (I can PR that).

@maltegrosse
Copy link
Owner

@mullerch , thanks for explanation.
2. is a valid point - especially if you wanna use that types in your further application logic.

Sure, more than welcome to do a PR. But please consider all structs (I think 19)

Thank you.

PS thanks (wifx) for the network manager golang library - was my inspiration for getting started with go and modem manager...

@mullerch
Copy link
Contributor Author

mullerch commented Dec 5, 2023

Credits for gonetworkmanager have to go to https://github.com/BellerophonMobile/ who initially made that library

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 a pull request may close this issue.

2 participants