Add battery IORegistry and sysmontap cpuusage #475

kvs-coder · 2024-09-10T12:51:03Z

No description provided.

danielpaulus

@kvs-coder do you need some help with it or why is it drafT? :-D

kvs-coder · 2024-10-14T21:06:01Z

@kvs-coder do you need some help with it or why is it drafT? :-D

it is draft because I started working on this, but then needed to switch context, and then had vacation :D

I'm resuming working on this to provide the possibility to read device's stats (like Xcode does, but without beautiful visual representation, just cold numbers)

If you have already some insights regarding this, I'm all ears :)

ios/instruments/instruments_sysmontap.go

ios/dtx_codec/channel.go

ios/instruments/instruments_deviceinfo.go

main.go

ios/instruments/instruments_sysmontap.go

dmissmann · 2024-11-01T11:24:52Z

ios/dtx_codec/channel.go

+	if msg.PayloadHeader.MessageType == UnknownTypeOne {
+		d.mutex.Unlock()
+		d.messageReceiver.msgChannel <- msg
+
+		return
+	}


Dispatcher implementations are getting those messages already atm, and we would change that with this filter here. Lets continue to send them to the Dispatcher, in our case now the GlobalDispatcher.

Since GlobalDispatcher only handles very few messages, we would need something that forwards it from there, or allow to pass in a custom Dispatcher for the global channel

according to the logs, it doesn't look like that it handles few messages, it is enormous amount of them, I tried already putting the logic in GlobalDispatcher only :D

it results in the problem here:

1 136 129 1 137 129 1 138 129 1 139 129 1 140 129 1 141 129 1 142 129 1 143 129 1 144 129 1 145 129 1 146 129 1 147 129 1 148 129 1 149 129 1 150 129 1 151 129 1 152 129 1 153 129 1 154 129 1 155 129 1 156 129 1 157 129 1 158 129 1 159 129 1 160 129 1 161 129 1 162 129 1 163 129 1 164 129 1 165 129 1 166 129 1 167 129 1 168 129 1 169 129 1 170 129 1 171 129 1 172 129 1 173 129 1 174 129 1 175 129 1 176 129 1 177 129 1 178 129 1 179 129 1 180 129 1 181 129 1 216 3 217 3 218 3 219 3 220 3 221 3 222 3 223 3 224 3 225 3 226 3 227 3 228 3 229 3 230 3 231 3 232 3 233 3 234 3 235 3 236 3 237 3 238 3 239 3 240 3 241 3 242 3 243 3 244 3 245 3 246 3 247 3 248 3 249 3 250 3 251 3 252 3 253 3 254 3 255 4 0 4 1 4 2 4 0 0 0 0 0 0 4 2 0 0 0 0 0 0 126 189 0 0 0 0 0 0 0 0 0 0 0 0 0 2 200 208]","level":"fatal","msg":"Start call error","time":"2024-11-01T10:09:30-05:00"}

you generally receive infinite bytes stream and then ends up with the error. That is why I provided the solution as it is now in the MR :)

I checked, with this current code, it has no negative effect on our side executions as this sysmontap gets only desired message and then we close the connection and the channel

I checked, with this current code, it has no negative effect on our side executions as this sysmontap gets only desired message and then we close the connection and the channel

Since UnknownTypeOne was added during the work of XCUITest for iOS 17 I strongly assume that a message with that type is sometimes sent during a test execution. And if that happens, we would block here forever as d.messageReceiver.msgChannel is nil. Pretty much any occasion of a message of that type would block forever, unless something calls Channel.Receive, but that would also only help on the global channel.

That problem is solved with having it in the Dispatcher interface as unhandled messages are only thrown away there.

We would need something in this method

go-ios/ios/dtx_codec/connection.go

Line 91 in 6fa8b80

func (g GlobalDispatcher) Dispatch(msg Message) {

that forwards the messages that were not handled in this method to another Dispatcher (or allow to set a customer Dispatcher on the global channel)

alright, the latest commit should safely avoid potential issues with XCTests

dmissmann · 2024-11-05T04:59:24Z

ios/instruments/instruments_sysmontap.go

+)
+
+type sysmontapMsgDispatcher struct {
+	channel chan dtx.Message


Suggested change

channel chan dtx.Message

messages chan dtx.Message

dmissmann · 2024-11-05T05:00:11Z

ios/instruments/instruments_sysmontap.go

+
+// Close closes up the DTX connection
+func (s *sysmontapService) Close() error {
+	return s.conn.Close()


Suggested change

return s.conn.Close()

close(s.msgDispatcher.messages)

return s.conn.Close()

dmissmann · 2024-11-05T05:08:44Z

ios/instruments/instruments_sysmontap.go

+		return SysmontapMessage{}, err
+	}
+
+	for {


if you only want to read a single value here the for loop isn't needed

unfortunately I really need this loop, as I previously highlighted, the global channel is like a garbage can and a lot of messages come there. The very first message is not addressing to the CPU data. If I remove the loop, the method just returns SysmontapMessage{}

ah right, missed the continue in there

dmissmann · 2024-11-05T05:11:39Z

ios/instruments/instruments_sysmontap.go

+		case <-time.After(30 * time.Second):
+			return SysmontapMessage{}, fmt.Errorf("exceeded waiting time message")


The timeout is not necessary. As long as the connection is alive, we should simply wait for a value here. And by adding the close for the channel from a comment above we can change case msg := <-s.msgDispatcher.channel: to case msg, ok := <-s.msgDispatcher.channel: and then the read would immediately return after the close with ok: false

dmissmann · 2024-11-05T05:24:42Z

ios/instruments/system_monitor.go

+func (s *systemMonitor) GetCPUUsage() (SysmontapMessage, error) {
+	sysAttrs, err := s.deviceInfoService.systemAttributes()
+	if err != nil {
+		return SysmontapMessage{}, err
+	}
+
+	procAttrs, err := s.deviceInfoService.processAttributes()
+	if err != nil {
+		return SysmontapMessage{}, err
+	}
+
+	err = s.sysmontapService.setConfig(procAttrs, sysAttrs)
+	if err != nil {
+		return SysmontapMessage{}, err
+	}
+
+	sysmontapMsg, err := s.sysmontapService.start()
+	if err != nil {
+		return SysmontapMessage{}, err
+	}
+
+	return sysmontapMsg, nil
+}


This has still the same problem as the version from last week or so. To get a single CPU sample all this setup has to be done. How do you get a second sample without doing this again? As I mentioned in the older comment, you should be able to get a continuous stream of CPU samples on a single connection, not just one.

Why not move this to NewSystemMonitor? Without that being successful the systemMonitor can't do anything, right? Then we can do this as part of creating it. And the call err := s.channel.MethodCallAsync("start") should also happen there then.

dmissmann · 2024-11-05T05:28:42Z

ios/instruments/instruments_sysmontap.go

+}
+
+// Creates a new sysmontapService
+func newSysmontapService(device ios.DeviceEntry) (*sysmontapService, error) {
+	msgDispatcher := newSysmontapMsgDispatcher()
+	dtxConn, err := connectInstrumentsWithMsgDispatcher(device, msgDispatcher)
+	if err != nil {
+		return nil, err
+	}
+
+	processControlChannel := dtxConn.RequestChannelIdentifier(sysmontapName, loggingDispatcher{dtxConn})
+
+	return &sysmontapService{channel: processControlChannel, conn: dtxConn, msgDispatcher: msgDispatcher}, nil
+}
+
+// Close closes up the DTX connection
+func (s *sysmontapService) Close() error {
+	return s.conn.Close()
+}
+
+// start sends a start method call async and waits until the cpu info & stats come back
+// the method is a part of the @protocol DTTapAuthorizedAPI
+func (s *sysmontapService) start() (SysmontapMessage, error) {


The important part here is not that start is getting sent (which shouldn't happen here anyways as mentioned in the comment about creating the systemMonitor), you are reading a sample, or message. That's what should be in the name

dmissmann · 2024-11-05T05:30:50Z

ios/instruments/system_monitor.go

+
+import "github.com/danielpaulus/go-ios/ios"
+
+type systemMonitor struct {


What's the reason for a separate type? Both, systemMonitor and sysmontapService contain something about CPU samples

dmissmann · 2024-11-05T05:31:52Z

ios/dtx_codec/connection.go

 // Dispatch prints log messages and errors when they are received and also creates local Channels when requested by the device.
 func (g GlobalDispatcher) Dispatch(msg Message) {
 	SendAckIfNeeded(g.dtxConnection, msg)
 	if msg.Payload != nil {
 		if requestChannel == msg.Payload[0] {
 			g.requestChannelMessages <- msg
 		}
+		if msg.PayloadHeader.MessageType == UnknownTypeOne {


I would simply move this at the end of this method without any further checks, everything that was not handled here already gets forwarded. If the receiver of this only needs a specific type of messages, it should discard them itself

ios/instruments/instruments_sysmontap.go

dmissmann · 2024-11-05T09:01:27Z

ios/dtx_codec/connection.go

+func (dtxConn *Connection) Dispatch(msg Message) {
+	msgDispatcher := dtxConn.MessageDispatcher
+	if msgDispatcher != nil {
+		log.Debugf("msg dispatcher found: %v", reflect.TypeOf(msgDispatcher))


there is a format specifier that you can use so that you don't have to go through the reflect library

Suggested change

log.Debugf("msg dispatcher found: %v", reflect.TypeOf(msgDispatcher))

log.Debugf("msg dispatcher found: %T", msgDispatcher)

danielpaulus reviewed Oct 14, 2024

View reviewed changes

fish-sauce force-pushed the vk-sysmontap branch from 7391a7a to 67ba804 Compare October 17, 2024 22:11

kvs-coder changed the title ~~Draft: sysmontap~~ Draft: Battery temperature Oct 17, 2024

fish-sauce force-pushed the vk-sysmontap branch 2 times, most recently from b94c3df to 0260f48 Compare October 18, 2024 13:21

kvs-coder changed the title ~~Draft: Battery temperature~~ Add Battery IORegistry Oct 18, 2024

fish-sauce force-pushed the vk-sysmontap branch 4 times, most recently from 3599c87 to 1ada922 Compare October 23, 2024 14:45

kvs-coder changed the title ~~Add Battery IORegistry~~ Add battery IORegistry and sysmontap cpuusage Oct 23, 2024

fish-sauce force-pushed the vk-sysmontap branch from 72b018f to 4430681 Compare October 24, 2024 14:00

fish-sauce added 3 commits October 24, 2024 09:11

Add battery ioregistry stats

6a95c27

Add sysmontap cpu usage stats

456904f

Add doc

8567537

fish-sauce force-pushed the vk-sysmontap branch from 4430681 to 8567537 Compare October 24, 2024 14:13

dmissmann requested changes Nov 1, 2024

View reviewed changes

fix: safe type assertions

dfbcce0

fish-sauce force-pushed the vk-sysmontap branch from 15f150d to 0c0712c Compare November 1, 2024 15:36

fix: add system monitor wrapper

58ba2d7

fish-sauce force-pushed the vk-sysmontap branch from 0c0712c to 58ba2d7 Compare November 1, 2024 15:39

hazmi-e205 mentioned this pull request Nov 2, 2024

Is there a function to check CPU/memory usage? #433

Open

fish-sauce force-pushed the vk-sysmontap branch 4 times, most recently from 5de862a to c68ba7e Compare November 4, 2024 18:32

fix: simplify to just channel

9b48bb4

fish-sauce force-pushed the vk-sysmontap branch 2 times, most recently from eb5b7d4 to 5785d14 Compare November 4, 2024 21:54

Dispatch to dedicated Dispatcher in Connection

83b2403

fish-sauce force-pushed the vk-sysmontap branch from 5785d14 to 83b2403 Compare November 4, 2024 22:11

dmissmann reviewed Nov 5, 2024

View reviewed changes

fish-sauce added 2 commits November 5, 2024 05:55

fix: close channel

a7e61d2

fix: deliver CPU sample continuosly

d8b2609

fish-sauce force-pushed the vk-sysmontap branch from 6ab38d6 to d8b2609 Compare November 5, 2024 12:54

Specify sampling rate

65b3915

dmissmann approved these changes Nov 5, 2024

View reviewed changes

Merge branch 'main' into vk-sysmontap

0cd587c

diegoperini merged commit 1789ded into danielpaulus:main Nov 5, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add battery IORegistry and sysmontap cpuusage #475

Add battery IORegistry and sysmontap cpuusage #475

kvs-coder commented Sep 10, 2024

danielpaulus left a comment

kvs-coder commented Oct 14, 2024 •

edited

Loading

dmissmann Nov 1, 2024

kvs-coder Nov 1, 2024 •

edited

Loading

dmissmann Nov 4, 2024

kvs-coder Nov 4, 2024

dmissmann Nov 5, 2024

dmissmann Nov 5, 2024

dmissmann Nov 5, 2024

kvs-coder Nov 5, 2024

dmissmann Nov 5, 2024

dmissmann Nov 5, 2024

dmissmann Nov 5, 2024

dmissmann Nov 5, 2024

dmissmann Nov 5, 2024

dmissmann Nov 5, 2024

dmissmann Nov 5, 2024

	return s.conn.Close()
	close(s.msgDispatcher.messages)
	return s.conn.Close()

		case <-time.After(30 * time.Second):
		return SysmontapMessage{}, fmt.Errorf("exceeded waiting time message")


		import "github.com/danielpaulus/go-ios/ios"

		type systemMonitor struct {

	log.Debugf("msg dispatcher found: %v", reflect.TypeOf(msgDispatcher))
	log.Debugf("msg dispatcher found: %T", msgDispatcher)

Add battery IORegistry and sysmontap cpuusage #475

Add battery IORegistry and sysmontap cpuusage #475

Conversation

kvs-coder commented Sep 10, 2024

danielpaulus left a comment

Choose a reason for hiding this comment

kvs-coder commented Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

kvs-coder Nov 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kvs-coder commented Oct 14, 2024 •

edited

Loading

kvs-coder Nov 1, 2024 •

edited

Loading