I don't like the idea. If we do this then we need to have code that handles plugin deps and makes sure that dependant plugins are loaded before the plugin that needs them.I see the problem here... Couldn't this be handled by using events? If the receiver plugin is not loaded the the events simply get to nirvana and the plugin detects it somehow over a timeout (wild speculations here)...
This is complex and error prone.
Why don't you just extend the cpu registers window with the needed functionality and the debugger api to match the needed features?I am not happy with extending the current plugin with the features, because i would like to make something like this:
I'll be happy to guide you, review and commit the changes.
and for this i need a .svd file parser and so on.What is a svd file?
I am not happy with extending the current plugin with the features, because i would like to make something like this:Do you need this to be a tree ctrl?
(http://www.atmel.com/Images/io_view.jpg)
This would bloat the plugin and if someone is not using the embedded stuff this is not needed...It could be enabled per configuration and not on by default.
An other possibility would be to extend the watch parsing (squirrel) with the ability to modify the controls....Don't do this, the squirrel code in the debugger plugin will be removed as soon as I finish merging the sqrat stuff.
What is a svd file?for ex. http://www.keil.com/pack/doc/cmsis/svd/html/svd__example_pg.html The file describes the internal periphery register structure of arm and other µC (i know the files also for atmega/xmega)..
Do you need this to be a tree ctrl?i don't know the UI yet.... some kind of property grid, it should have the possibility to use drop down menus,check boxes and text fields. Custom controls like leds (red dots) would also be nice... A search field would also be needed, because there can be ton's of register and flag names
By bloating i mean code/ binary size... More things to maintain for the main dev team ;) Of corse it is possible to add it at the core and make it configurable, but i don't like the idea to add something to the main plugin what is not needed for the core. Because there is also potential for more logging/debugging like:QuoteQuote from: BlueHazzard on Yesterday at 18:54:32It could be enabled per configuration and not on by default.
This would bloat the plugin and if someone is not using the embedded stuff this is not needed...
Sad... i think there would be a lot potential. Specially in automation of gdb things and embedded stuff (see previous image)...QuoteQuote from: BlueHazzard on Yesterday at 18:54:32Don't do this, the squirrel code in the debugger plugin will be removed as soon as I finish merging the sqrat stuff.
An other possibility would be to extend the watch parsing (squirrel) with the ability to modify the controls....
for ex. http://www.keil.com/pack/doc/cmsis/svd/html/svd__example_pg.html The file describes the internal periphery register structure of arm and other µC (i know the files also for atmega/xmega)..I think some of the embedded forks of cb has a parser for this added to the wizard.
i don't know the UI yet.... some kind of property grid, it should have the possibility to use drop down menus,check boxes and text fields. Custom controls like leds (red dots) would also be nice... A search field would also be needed, because there can be ton's of register and flag namesUnfortunately the propgrid is the best ui for this, but I'm not sure I like this control. Especially the migration from 2.8 to 3.x. :(
By bloating i mean code/ binary size... More things to maintain for the main dev team ;) Of corse it is possible to add it at the core and make it configurable, but i don't like the idea to add something to the main plugin what is not needed for the core.Don't worry too much about this. The cpu registers window needs improvements. If you make them general enough it might be good for all users.
Because there is also potential for more logging/debugging like:This is totally separate topic and probably a lot more work and it is better to discuss it later, when the cpu registers problem is solved.
(http://www.visualmicro.com/pics/Arduino%20Debug%20Auto%20Reports%20Config%20Example%20Small.png)
I don't think this functionality should be in the core. But if you want to use codeblocks (i think there is no really nice, cross platform and fast IDE for debugging embedded systems out there) for embedded development you "need" this things. It would simply be nice to have somehow an interface to gdb...
Sad... i think there would be a lot potential. Specially in automation of gdb things and embedded stuff (see previous image)...Now we have python printers directly in gdb, which do better job handling the watches. Our scripts for the watches need lots of work, I'm not sure how to do even. We need a way to execute a command, wait for its output, parse it, execute another command and so on.
I think some of the embedded forks of cb has a parser for this added to the wizard.I have looked into emBitz, but it is somehow closed source and won't run on linux...
Don't worry too much about this. The cpu registers window needs improvements. If you make them general enough it might be good for all users.
And there people which will be happy to use something like cb, instead of the vendors ides.
And for the record I don't like the idea to have two separate cpu registers windows - one for host and one for embedded development.The thing is this: periphery register are not cpu registers... (like the program counter or the cpu status register) this register are read with the info registers command of gdb.
I have looked into emBitz, but it is somehow closed source and won't run on linux...Does it still look like code::blocks 10.05?
It is possible to merge this two register views, but i don't know if this is a wide spread opinion...Why not?
Why not?Well lets try it...
If the outcome is that i create a new plugin a interface to the gdb plugin is needed:
1) I can think of a universal interface
1.1) like "wxString SendCommandToCurrentActiveDebugger(wxString command)" what is blocking, takes a string as command and returns a string as response
1.2) Or a cb event? Something like request debugger command... with a global answer event, for all plugins...
2) It should be future proof, if there is at some point the transmission to the mi interface it should still work
3) like on top
If the outcome of this discussion is to add this functionality to the cpu register window there are some things to clear:1. This is fine, but this is only GDB's plugin UI. Other plugins will create separate tabs, so this is a bit grey area. I never got to make the per project UI unified. :(
1) Where store the path to the SVD file. There is the need of a project settings place. Probably the "Debugger" tab of the Project options
2) What control has to be used. I think the property grid is needed!
3) Is there the will of the devs to integrate my work in the code base (this point is always valid)
4) Because the cpu register and peripheral register are called from different gdb commands there is a lot of rework in the current gdb driver
5) This will add a "huge" xml parser for svd files, that has to be maintained, to the codeblocks code base
Multiple watch windows aren't implemented because wxAUI doesn't support moving one pane to a random notebook, so there is no UI space left. Unfortunately I don't think wx will add this feature any time soon, so we'll have to probably think about some hacky solution. :(Multiple tabs for the watches window? Anyway, this is not part of this discussion, i wanted only show a second use case.
For the memory watches - just add the appropriate flags in the GDBWatch class and do separate processing based on these flags.So you think about modifying the cbWatch class? This would be ok, what about to store the owner of the watch?
I don't understand all the talk about the ExamineMemory dialogs...on embedded development it is quite handy
Keep in mind that working on the current gdb plugin will most probably be a waste of your time...I don't think so:
Do you have some branch somewhere I can look at?i have started some work here: https://github.com/bluehazzard/codeblocks_sf on the new_cpu_register_window_01 branch
One note don't add non generic stuff to sdk classes. I can see you're adding some svd fields in the project. This is not a good idea. Add some generic storage and provide a way to define multiple parsers. As far as I can see svd is arm only and thus it is not generic.
class cbMemoryWatch
{
uint64_t m_address; // start address of the memory watch
uint64_t m_count; // count of bytes
vector<uint8_t> m_data; // The actual data
bool IsChanged() const;
const std::vector<uint8_t>& GetData()
}
virtual cb::shared_ptr<cbMemoryWatch> AddMemoryWatch(const uint64_t address, const uint64_t count) = 0;
virtual void DeleteMemoryWatch(cb::shared_ptr<cbMemoryWatch> watch) = 0;
virtual bool HasMemoryWatch(cb::shared_ptr<cbMemoryWatch> watch) = 0;
virtual bool SetMemoryWatchValue(cb::shared_ptr<cbMemoryWatch> watch, const std::vector<uint8_t>& value) = 0;
virtual void UpdateMemoryWatch(cb::shared_ptr<cbMemoryWatch> watch) = 0;
Are these things always read using the x command? For all controllers or this is stored in the svd?As far as i can tell all big microcontroller map their periphery register to ram, so it should always be possible to read them from ram with x.
Is it possible for these things to have sub-fields? Or do they store just a single value?I can not find any nice image but you can get a idea from here: http://www.forward.com.au/BluetoothLedDriver/PINB_PB2.JPG for AVR
Do we need to display them in the UI like we display a C/C++ struct?
Lets see if we can reuse the watches functionality for these things.i am open for ideas, but keep in mind that it should be fast and the data should be as raw as possible...
The cbMemoryWatch proposal is not a good one because it expands the API too much.I can understand this point to some extend if there are a lot external plugins, but codeblocks, at the moment, is not a application with tons of external plugins (at least i am not aware of them), so breaking the API should not be a big deal.
edit: What is the cheapest way I can get one such controller+jtag debugger for experimentation?probably a NUCLEO-F103RB, nucleo-f091rc or a other stm discovery board (ARM controller from STMicroelectronics ) with around 15€. The debugger (SWD) is included and you get the possibility to make a full j-link ( https://www.segger.com/products/debug-probes/j-link/models/other-j-links/st-link-on-board/ ) out of it. Note: i have yet to try to use the stm controller to work in codeblocks. At the moment i use an arduino due and a j-link from segger (Atmel ARM controller)
API Breakage is not problematic. I just don't want to make the API big. I prefer if we could re-use as much as possible.But you have to give people a possibility to work with it.
Do you expect to add "watches" for all supported "registers" from the file at once or do you add them one by one (the ones you need at the moment)?Probably as needed. It would be nice to make the first read out to read all registers but this can be a lot data (11kByte) and so it is probably better to read only the current open registers. This should be as fast as possible...
Can you find some example svd file and then show the UI we should generate? So we can have something more concrete example we could discuss.I updated my test app to populate a property grid. This is by far not complete (1), but you will get a idea (img is attached)
If you read every register with a single x command then the performance will be rather bad (similarly bad as if you add many watches).
<name>UART</name>
<version>6418H</version>
<description>Universal Asynchronous Receiver Transmitter</description>
<prependToName>UART_</prependToName>
<baseAddress>0x400E0800</baseAddress>
<addressBlock>
<offset>0</offset>
<size>0x128</size>
<usage>registers</usage>
</addressBlock>
<interrupt>
<name>UART</name>
<value>8</value>
</interrupt>
<registers>
<name>CR</name>
<description>Control Register</description>
<addressOffset>0x00000000</addressOffset>
<size>32</size>
<access>write-only</access>
<fields>
<field>
<name>RSTRX</name>
<description>Reset Receiver</description>
<bitOffset>2</bitOffset>
<bitWidth>1</bitWidth>
<access>write-only</access>
</field>
<field>
<name>RSTTX</name>
<description>Reset Transmitter</description>
<bitOffset>3</bitOffset>
<bitWidth>1</bitWidth>
<access>write-only</access>
</field>
<field>
<name>RXEN</name>
<description>Receiver Enable</description>
<bitOffset>4</bitOffset>
<bitWidth>1</bitWidth>
<access>write-only</access>
</field>
<field>
<name>RXDIS</name>
<description>Receiver Disable</description>
<bitOffset>5</bitOffset>
<bitWidth>1</bitWidth>
<access>write-only</access>
</field>
<field>
<name>TXEN</name>
<description>Transmitter Enable</description>
<bitOffset>6</bitOffset>
<bitWidth>1</bitWidth>
<access>write-only</access>
</field>
<field>
<name>TXDIS</name>
<description>Transmitter Disable</description>
<bitOffset>7</bitOffset>
<bitWidth>1</bitWidth>
<access>write-only</access>
</field>
<field>
<name>RSTSTA</name>
<description>Reset Status Bits</description>
<bitOffset>8</bitOffset>
<bitWidth>1</bitWidth>
<access>write-only</access>
</field>
</fields>
</register>
<register>
<name>MR</name>
<description>Mode Register</description>
<addressOffset>0x00000004</addressOffset>
<size>32</size>
<access>read-write</access>
<resetValue>0x00000000</resetValue>
<fields>
<field>
<name>PAR</name>
<description>Parity Type</description>
<bitOffset>9</bitOffset>
<bitWidth>3</bitWidth>
<access>read-write</access>
<enumeratedValues>
<enumeratedValue>
<name>EVEN</name>
<description>Even Parity</description>
<value>0x0</value>
</enumeratedValue>
<enumeratedValue>
<name>ODD</name>
<description>Odd Parity</description>
<value>0x1</value>
</enumeratedValue>
<enumeratedValue>
<name>SPACE</name>
<description>Space: parity forced to 0</description>
<value>0x2</value>
</enumeratedValue>
<enumeratedValue>
<name>MARK</name>
<description>Mark: parity forced to 1</description>
<value>0x3</value>
</enumeratedValue>
<enumeratedValue>
<name>NO</name>
<description>No Parity</description>
<value>0x4</value>
</enumeratedValue>
</enumeratedValues>
</field>
<field>
<name>CHMODE</name>
<description>Channel Mode</description>
<bitOffset>14</bitOffset>
<bitWidth>2</bitWidth>
<access>read-write</access>
<enumeratedValues>
<enumeratedValue>
<name>NORMAL</name>
<description>Normal Mode</description>
<value>0x0</value>
</enumeratedValue>
<enumeratedValue>
<name>AUTOMATIC</name>
<description>Automatic Echo</description>
<value>0x1</value>
</enumeratedValue>
<enumeratedValue>
<name>LOCAL_LOOPBACK</name>
<description>Local Loopback</description>
<value>0x2</value>
</enumeratedValue>
<enumeratedValue>
<name>REMOTE_LOOPBACK</name>
<description>Remote Loopback</description>
<value>0x3</value>
</enumeratedValue>
</enumeratedValues>
</field>
</fields>
</register>
With the current implementation of the watches, every watch saved in the interna of the debugger is mirrored to the watches window. For this purpose this is not wanted...Do you mean that there are GDBWatch and WatchesProperties objects? The cbWatch objects are managed by the debugger, because it knows best what additional data it needs for them to keep them up-to-date.
cbWatch* cbDebuggerPlugin::AddPeripheralRegister(uint64_t address, uint64_t size, const wxString &id (or fullname) )
Probably as needed. It would be nice to make the first read out to read all registers but this can be a lot data (11kByte) and so it is probably better to read only the current open registers. This should be as fast as possible...There is no way to be notified of changes in the memory locations (hw watch points are too limited for this purpose).
Quote from: BlueHazzard on Yesterday at 02:19:38i looked again in the code and now i think i understand what is going on:
With the current implementation of the watches, every watch saved in the interna of the debugger is mirrored to the watches window. For this purpose this is not wanted...
Do you mean that there are GDBWatch and WatchesProperties objects? The cbWatch objects are managed by the debugger, because it knows best what additional data it needs for them to keep them up-to-date.
For this new cbWatch type we could add something like:Again, i would not make this (the name AddPeripheralRegister) so special... Just lets read a count of bytes from some memory location. This would allow us to make this way more flexible...
Code: [Select]
cbWatch* cbDebuggerPlugin::AddPeripheralRegister(uint64_t address, uint64_t size, const wxString &id (or fullname) )
There is no way to be notified of changes in the memory locations (hw watch points are too limited for this purpose).Let me describe the procedure:
So if you want all "registers" to be updated at every "next line" or "step into" command then this will be massively slow.
In the watches window there is functionality that makes it possible to mark that you want only certain watches to be auto updated. And there is manual update command per watch. Similar mechanism should be use here, too.
Again, i would not make this (the name AddPeripheralRegister) so special... Just lets read a count of bytes from some memory location. This would allow us to make this way more flexible...What do you mean by flexible here? I don't care much about the exact name, but I don't like MemoryWatch, because it is too similar to data breakpoint and tracepoints. Which are different things.
What do you mean by flexible here? I don't care much about the exact name, but I don't like MemoryWatch, because it is too similar to data breakpoint and tracepoints. Which are different things.It would be possible for other plugins to read out memory from the microcontroller and use it for own things (for example https://www.segger.com/products/debug-probes/j-link/technology/real-time-transfer/general-information/#tab-15668-1 this is only a general idea, there are countless other things, like generating memory maps from MMU and DMA and so on) In general, if you can read the ram from a microcontroller you can do a hell lot of things. With this kind of watch it would be possible to do all this things from a plugin.
About step 1: How do you select the svd file? The user selects it every time the window is being opened? It is stored in a project/target/the window and the window resets it every time new debug session is started?Of course it should be saved at project or target level. But this can be done from the plugin and codeblocks provides a api for this, so no problems here ( as far as i know a plugin can add notebooks to the Project->Settings dialog, at least the gdb plugin does it and so there is also a place for the setting entry). The svd file is parsed on load of the project (and when the file is changed in the options of corse). The window is populated at the opening of the debugger, with all elements. The window provides a search control so the user can find the needed register pretty fast. The update of the data is then a other problem, but there is the possibility to optimize, for example find the best size between count of watches and size of a singe watch.
About Step 5: linking the expanding of the property and updating the values is not a good idea. At least for the watches it is a terrible idea,Why is this a bad idea? Only get the data from the debugger as you need it, because from my experience the problem is codeblocks<->gdb and not gdb or codeblocks at program level. I think the mi gdb plugin does this and it is fast and reliable. You can cache the values and only update if the debugger was running in the mean time... In any way it is faster then updating the whole register map (if the map is 11kByte big, on smaller targets it may be a possibility. Don't optimize until you know where the problem is...)
With this kind of watch it would be possible to do all this things from a plugin.I really doubt it. Especially if you want to transfer tons of data.
At the moment i would like to implement it and for this i need to know what api the devs would support.It will be good if you could try the minimal version of the watch idea. In order to see if it is worth pursuing. Nothing fancy and complex on the UI part. Get adding (use some hardcoded path to a svd file for example) and updating watches to work. My original thought was to put the register watches in the watches window, because it will make it easier to experiment with it.
For optimization is later time, if the bottleneck is found.The problem with api design is that you should design for performance up front. It is very hard to make a system really performant if the api is not optimal.
Why is this a bad idea?It creates delays for the user when you need the data. I guess here it won't be that bad and it could be disabled if it creates too many ux problems. I'm not sure I like the similar implementation in the gdb/mi plugin.
...because from my experience the problem is codeblocks<->gdb and not gdb or codeblocks at program level.Sometimes gdb is slow, because it has tons of data to process and produces small amount of data.
In any way it is faster then updating the whole register map (if the map is 11kByte big, on smaller targets it may be a possibility...)This could be done in the gdb plugin. Because it is its job to iterate the register-watches and decide what commands to issue. It might be possible to merge different memory ranges. If they overlap or next to each other.
enum cbWatchType
{
cbWatchType_SYMBOL,
cbWatchType_MEMORY
};
virtual void AddMemoryRange(uint64_t address, uint64_t size, const wxString &id ) = 0;
cbWatchType GetWatchType() {return m_watchType;};
private:
cbWatchType m_watchType;
void GDBWatch::AddMemoryRange(uint64_t address, uint64_t size, const wxString &id )
{
m_symbol = wxString::Format(wxT("/%ulub %lx"), size, address);
m_watchType = cbWatchType_MEMORY;
}
void GDBWatch::SetSymbol(const wxString& symbol)
{
m_symbol = symbol;
m_watchType = cbWatchType_SYMBOL;
}
GdbCmd_Watch(DebuggerDriver* driver, cb::shared_ptr<GDBWatch> watch) :
DebuggerCmd(driver),
m_watch(watch)
{
wxString type;
wxString symbol;
m_watch->GetSymbol(symbol);
m_watch->GetType(type);
if(m_watch->GetWatchType == cbWatchType_MEMORY)
{
m_Cmd << "x " << symbol;
return;
}
// old code from here on
type.Trim(true);
type.Trim(false);
m_Cmd = static_cast<GDB_driver*>(m_pDriver)->GetScriptedTypeCommand(type, m_ParseFunc);
if (m_Cmd.IsEmpty())
{
ParseGDBWatchValue
DeleteMemoryRange and HasMemoryRange seem redundant...i agree with that.. This popped up my mind as soon as i pushed the code
Also I'm not sure it is a good idea to use GDBWatch as a class and to store the command in the symbol field or the result as a string in the value.This could easily be made, and honestly i would prefer it. But the result has to be stored in the value member as string, because otherwise the cbWatch has to be modified, or how do you think to get the result from outside of the gdb plugin? One possibility would be to add a function
bool GetMemoryRangeValue(cb:shared_ptr<cbWatch> watch, std::vector<char> &data)
This could easily be made, and honestly i would prefer it. But the result has to be stored in the value member as string, because otherwise the cbWatch has to be modified, or how do you think to get the result from outside of the gdb plugin?It depends what you want to do with the data. Do you know what operations you want to do?
One possibility would be to add a functionDo not go this route, please.Codeto cbDebuggerPlugin, but this seems not intuitive if the normal use of cbWatch is GetValue().bool GetMemoryRangeValue(cb:shared_ptr<cbWatch> watch, std::vector<char> &data)
cb::shared_ptr<cbWatch> cbDebuggerPlugin::AddMemoryRange(uint64_t address, uint64_t size, const wxString &id ) = 0;
cb::shared_ptr<cbWatch> m_watch = dbg_plugin->AddMemoryRange(0x400000, 16, wxEmtyString );
// Run the debugger
wxString tmp;
m_watch->GetValue(tmp);
size_t lengthOfData = tmp.size();
char* memoryContetn = new char[ lengthOfData ];
memcpy(memoryContent, tmp.To8BitData(), 16);
// yay: ram content in memoryContent for your free use and interpretation
// i know there is a uint16_t @ 0x400002
uint16_t myNeededValue = 0;
memcpy(&myNeededValue, memoryContent+2, 2);
std::vector<cb::shared_ptr<GDBMemoryRangeWatch> > m_memoryRange;
void DebuggerGDB::DeleteWatch(cb::shared_ptr<cbWatch> watch)
bool DebuggerGDB::HasWatch(cb::shared_ptr<cbWatch> watch)
bool DebuggerGDB::IsMemoryRangeWatch(cb::shared_ptr<cbWatch> watch)
class GdbCmd_MemoryRangeWatch : public DebuggerCmd